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

Small style fixes to Installer Set #1985

Merged
merged 2 commits into from Aug 8, 2017
Merged

Small style fixes to Installer Set #1985

merged 2 commits into from Aug 8, 2017

Conversation

ghost
Copy link

@ghost ghost commented Aug 8, 2017

[-] Remove redundant comma from InstallerSet#local?

[-] Remove redundant comma from InstallerSet#local?
@@ -193,7 +193,7 @@ def load_spec name, ver, platform, source # :nodoc:
# Has a local gem for +dep_name+ been added to this set?

def local? dep_name # :nodoc:
spec, = @local[dep_name]
spec = @local[dep_name]
Copy link
Member

@bronzdoc bronzdoc Aug 8, 2017

Choose a reason for hiding this comment

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

Hey, thanks for contributing to rubygems :)

I don't think that comma is just for style purposes, that comma there ensure we get the spec part from @local[dep_name]

since @local[dep_name] can be equal to [spec, source]

Copy link
Author

Choose a reason for hiding this comment

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

Thank you for your comment. Maybe you can point on some places in the rubygems that can be useful for the newbie contributor like me to help this gem out?

Choose a reason for hiding this comment

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

If that's the case, this might be more clear:

spec, _source = @local[dep_name]

Or:

spec, _ = @local[dep_name]

[+] Add  throwaway variable _ to make intention of getting only the spec clearer.
@bronzdoc
Copy link
Member

bronzdoc commented Aug 8, 2017

Thanks!

@homu r+

@bronzdoc
Copy link
Member

bronzdoc commented Aug 8, 2017

@bundlerbot r+

@bundlerbot
Copy link
Collaborator

📌 Commit e13e2b7 has been approved by bronzdoc

@bundlerbot
Copy link
Collaborator

⌛ Testing commit e13e2b7 with merge 097222b...

bundlerbot added a commit that referenced this pull request Aug 8, 2017
Small style fixes to Installer Set

[-] Remove redundant comma from InstallerSet#local?
@bundlerbot
Copy link
Collaborator

☀️ Test successful - status-travis
Approved by: bronzdoc
Pushing 097222b to master...

@bundlerbot bundlerbot merged commit e13e2b7 into rubygems:master Aug 8, 2017
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.

None yet

4 participants