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

Added option to pass configs as args for all config files #12

Merged

Conversation

samarthkathal
Copy link
Contributor

Fixes #6

@samarthkathal
Copy link
Contributor Author

samarthkathal commented May 30, 2023

@DmitryTsepelev I think we should probably rename .rubocop-director.yml to .rubocop_director.yml as it goes well with all other config files (rubocop and rubocop_todo).

Please also have a look at the option naming and description I have decided to go with.
I'd like your input before I write tests for this PR. Thanks!

@DmitryTsepelev
Copy link
Owner

Overall looks good 🙂

I think we should probably rename .rubocop-director.yml to .rubocop_director.yml

makes sense

@samarthkathal samarthkathal marked this pull request as ready for review June 7, 2023 09:57
@samarthkathal
Copy link
Contributor Author

@DmitryTsepelev PR is ready for review. Please have a look.

Copy link
Owner

@DmitryTsepelev DmitryTsepelev left a comment

Choose a reason for hiding this comment

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

Couple of tweaks and we're good to go 🙂

lib/rubocop_director/commands/generate_config.rb Outdated Show resolved Hide resolved
p.on("--since=SINCE", "Specify date to start checking git history") do |since|
@since = since
end
p.on("--generate_config", "Generate default config based on .rubocop_todo.yml")
Copy link
Owner

Choose a reason for hiding this comment

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

I'd rather keep - version everywhere (--generate-config instead of --generate_config)

Copy link
Contributor Author

@samarthkathal samarthkathal Jun 12, 2023

Choose a reason for hiding this comment

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

Hey, I did some testing when I decided to go with - instead which I forgot to mention.
hyphen (-) and underscore (_) can be used interchangeably when using them with OptionParser.

ie in this case, bundle exec rubocop-director --generate-config == bundle exec rubocop-director --generate_config

The only difference between them is that args containing a hyphen are parsed into :'generate-config' whereas args without them can be directly used as symbols :generate_config

I also have a question about ruby symbols that I think you can help me with.
I know that (1) :'abc' and (2) :abc both are symbols. Ideally, there should not be any difference bw them. But I try to never use the first kind. I have an assumption that in the case of (1), a string is getting converted into a symbol. whereas (2) itself is a symbol. Am I correct in making this assumption? Also, do share if you know of any differences.

It is kind of confusing so please let me know if it does not make sense.
Thanks!

Copy link
Owner

Choose a reason for hiding this comment

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

hyphen (-) and underscore (_) can be used interchangeably when using them with OptionParser.

ah I didn't know that

Am I correct in making this assumption? Also, do share if you know of any differences.

I think you're right about the conversion (but maybe it's done in the "compile" time when they are used as literals)

@DmitryTsepelev DmitryTsepelev merged commit fb405f6 into DmitryTsepelev:master Jun 14, 2023
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.

Add option to pass config path to all related commands
2 participants