Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(GH-1889) Fixes Equals implementation of ConfigFileApiKeySettings #1890

Merged
merged 1 commit into from
Apr 16, 2021

Conversation

skel35
Copy link
Contributor

@skel35 skel35 commented Jul 28, 2019

Fixes Equals implementation of ConfigFileApiKeySettings.

This is the line which failed due to Equals not working correctly:

configFileSettings.ApiKeys.RemoveWhere(x => x.Source.is_equal_to(configuration.Sources));

Closes #1889.

@@ -43,7 +43,7 @@ public override bool Equals(object obj)
var item = (ConfigFileApiKeySetting) obj;

return (Source == item.Source)
&& (Key == item.Source);
&& (Key == item.Key);
Copy link
Member

Choose a reason for hiding this comment

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

Good catch! This came in as a PR and it appears this was missed on the review!

Copy link
Member

@ferventcoder ferventcoder left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Member

@ferventcoder ferventcoder left a comment

Choose a reason for hiding this comment

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

Okay, apologies on jumping the gun there - I do need the git commit to follow our CONTRIBUTING rules. Can you review that and provide a better commit message and body? Thanks in advance!

@skel35
Copy link
Contributor Author

skel35 commented Aug 5, 2019

Updated the commit message

@ferventcoder
Copy link
Member

I would see the git commit summary of this fix be Fix: Removal of ApiKey broken

@ferventcoder
Copy link
Member

This is ready to be pulled into stable!

@ferventcoder
Copy link
Member

Thanks for getting that updated.

@CLAassistant
Copy link

CLAassistant commented Jun 8, 2020

CLA assistant check
All committers have signed the CLA.

@jansohn
Copy link

jansohn commented Feb 25, 2021

@ferventcoder why is this still not merged? everything seems to be in place?

`remove_api_key` was previously not working properly. The API key string
did not get removed from the config file due to incorrect implementation
of Equals method  in ConfigFileApiKeySettings class.
Current commit fixes this issue.
@gep13 gep13 force-pushed the f/GH-1889-Fix-ApiKey-Remove branch from 731296f to e89ee92 Compare April 16, 2021 12:59
@gep13 gep13 changed the base branch from master to stable April 16, 2021 12:59
Copy link
Member

@gep13 gep13 left a comment

Choose a reason for hiding this comment

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

LGTM!

@gep13
Copy link
Member

gep13 commented Apr 16, 2021

@skel35 thanks again for taking the time to address this issue! Once the CI builds are completed, I will get this merged in.

@gep13 gep13 merged commit c157be0 into chocolatey:stable Apr 16, 2021
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.

Unable to remove API Key with choco apikey command
5 participants