Skip to content

Modify TestAppExample to add service#282

Open
KangLin wants to merge 1 commit into
frankosterfeld:mainfrom
KangLin:TestAppExample
Open

Modify TestAppExample to add service#282
KangLin wants to merge 1 commit into
frankosterfeld:mainfrom
KangLin:TestAppExample

Conversation

@KangLin

@KangLin KangLin commented Oct 10, 2025

Copy link
Copy Markdown
Contributor

No description provided.


void KeyChainClass::setService(const QString &service)
{
if(m_readCredentialJob) delete m_readCredentialJob;

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't do if (foo) delete foo, the if isn't necessary as deleting a nullptr is safe.

if(m_writeCredentialJob) delete m_writeCredentialJob;
if(m_deleteCredentialJob) delete m_deleteCredentialJob;

m_readCredentialJob = new QKeychain::ReadPasswordJob(service, this);

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to check all the implementations, but I see no strict reason why there shouldn't be a setService() setter... then this patch would become a lot simpler/ smaller.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nevermind, the code changed already to dynamic jobs anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants