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

Add bindir flag to pristine #2361

Merged
merged 1 commit into from Jul 30, 2018
Merged

Add bindir flag to pristine #2361

merged 1 commit into from Jul 30, 2018

Conversation

bronzdoc
Copy link
Member

Description:

closes #1997

I will abide by the code of conduct.

@@ -46,6 +46,12 @@ def initialize
options[:env_shebang] = value
end

add_option('-n', '--bindir DIR',
'Directory where binary files are',
Copy link
Member

Choose a reason for hiding this comment

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

should this be binstubs instead of binary files ?

Copy link
Contributor

Choose a reason for hiding this comment

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

add_option(:"Install/Update", '-n', '--bindir DIR',
'Directory where binary files are',
'located') do |value, options|
options[:bin_dir] = File.expand_path(value)
end

Copy link
Member

Choose a reason for hiding this comment

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

Even if it’s copy-pasted from elsewhere... the “bin” files RubyGems installs aren’t actually binary

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd agree. The terms 'binary' and 'binstubs' are used both as above and in docs. What's a better name? Maybe 'launch scripts', 'CLI scripts', 'scripts', etc. I don't know...

@ghost
Copy link

ghost commented Jul 22, 2018 via email

@bronzdoc
Copy link
Member Author

bronzdoc commented Jul 22, 2018

The reason the message is what it is, is because I wanted to be consistent with the install/update —bindir Flag description, I’ll change it to whatever we agree on.

@MSP-Greg
Copy link
Contributor

@bronzdoc`

I’ll change it to whatever we agree on.

Doubt I'm part of the 'we' (not complaining), but since you've matched install/update, I think it's as it should be. The issue of what to call these files should also account for (or fix) what's currently shown in the UI prompts, which I believe is 'executables'...

@bronzdoc
Copy link
Member Author

My opinion is that we should merge this and and open a new issue to discuss how this things should be named, then i could follow up with a PR that changes where appropriate. what you think? @segiddins @MSP-Greg @tjouan

@MSP-Greg
Copy link
Contributor

My opinion is that we should merge this and open a new issue...

Agreed...

@bronzdoc
Copy link
Member Author

@bundlerbot r+

@bundlerbot
Copy link
Collaborator

📌 Commit cc8c63b has been approved by bronzdoc

@bundlerbot
Copy link
Collaborator

⌛ Testing commit cc8c63b with merge af9a1a0...

bundlerbot added a commit that referenced this pull request Jul 30, 2018
Add bindir flag to pristine

# Description:

closes #1997

I will abide by the [code of conduct](https://github.com/rubygems/rubygems/blob/master/CODE_OF_CONDUCT.md).
@bundlerbot
Copy link
Collaborator

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

@bundlerbot bundlerbot merged commit cc8c63b into master Jul 30, 2018
@ghost
Copy link

ghost commented Jul 30, 2018 via email

@bronzdoc bronzdoc deleted the add_pristine_bindir branch September 12, 2018 16:13
@colby-swandale colby-swandale added this to the 2.8.0 milestone Sep 23, 2018
voxik added a commit to voxik/rubygems that referenced this pull request Oct 16, 2019
Since PR rubygems#2361, the `@bin_dir` is set in `Gem::Installer#initialize`,
while it was already properly set via preceeding `process_options` call.
voxik added a commit to voxik/rubygems that referenced this pull request Oct 16, 2019
Since PR rubygems#2361, the `@bin_dir` is set in `Gem::Installer#initialize`,
while it was already properly set via preceding `process_options` call.
voxik added a commit to voxik/rubygems that referenced this pull request Oct 16, 2019
Since PR rubygems#2361, the `@bin_dir` is set in `Gem::Installer#initialize`,
while it was already properly set via preceding `process_options` call.
@voxik voxik mentioned this pull request Oct 16, 2019
ghost pushed a commit that referenced this pull request Oct 16, 2019
2947: Refactor keyword argument test for Ruby 2.7 r=hsbt a=hsbt

# Description:

ruby/ruby@3e3cc08
______________

# Tasks:

- [ ] Describe the problem / feature
- [ ] Write tests
- [ ] Write code to solve the problem
- [ ] Get code review from coworkers / friends

I will abide by the [code of conduct](https://github.com/rubygems/rubygems/blob/master/CODE_OF_CONDUCT.md).


2950: Don't set `bin_dir` twice. r=hsbt a=voxik

Since PR #2361, the `@bin_dir` is set in `Gem::Installer#initialize` [[1]], while it was already properly set via preceding `process_options` call [[2]].


[1]: https://github.com/rubygems/rubygems/blob/6b2b09ac0ddf41c0824e1244aaa9c8120c3081c4/lib/rubygems/installer.rb#L199
[2]: https://github.com/rubygems/rubygems/blob/6b2b09ac0ddf41c0824e1244aaa9c8120c3081c4/lib/rubygems/installer.rb#L697

Co-authored-by: Nobuyoshi Nakada <nobu@ruby-lang.org>
Co-authored-by: Vít Ondruch <vondruch@redhat.com>
hsbt pushed a commit that referenced this pull request Nov 11, 2019
2947: Refactor keyword argument test for Ruby 2.7 r=hsbt a=hsbt

# Description:

ruby/ruby@3e3cc08
______________

# Tasks:

- [ ] Describe the problem / feature
- [ ] Write tests
- [ ] Write code to solve the problem
- [ ] Get code review from coworkers / friends

I will abide by the [code of conduct](https://github.com/rubygems/rubygems/blob/master/CODE_OF_CONDUCT.md).


2950: Don't set `bin_dir` twice. r=hsbt a=voxik

Since PR #2361, the `@bin_dir` is set in `Gem::Installer#initialize` [[1]], while it was already properly set via preceding `process_options` call [[2]].


[1]: https://github.com/rubygems/rubygems/blob/6b2b09ac0ddf41c0824e1244aaa9c8120c3081c4/lib/rubygems/installer.rb#L199
[2]: https://github.com/rubygems/rubygems/blob/6b2b09ac0ddf41c0824e1244aaa9c8120c3081c4/lib/rubygems/installer.rb#L697

Co-authored-by: Nobuyoshi Nakada <nobu@ruby-lang.org>
Co-authored-by: Vít Ondruch <vondruch@redhat.com>
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.

pristine --bindir
5 participants