clerk-sdk-ruby
clerk-sdk-ruby copied to clipboard
chore: Add code formatting
Nice one!
How are we planning to use this, when is the formatter supposed to run?
I think we should at least run it manually until we create GH action to run make PRs fail if there are any lint changes found. What do you think?
I think we should at least run it manually until we create GH action to run make PRs fail if there are any lint changes found. What do you think?
I think we should somehow enforce it from the start, so maybe consider adding a linter?
Ideally, formatting should run automatically.
Do you happen to know if there's any configuration that we can use for our editors that would perhaps format on every save? I've had success with "autoformat on save" using rufo in the past, perhaps it works similarly?
@gkats I found the following resources for rubocop linter:
- github action: added step in main.yml as part of this PR
- plugin for vim: https://github.com/ngmy/vim-rubocop
- plugin vscode: https://github.com/rubocop/vscode-rubocop
- i have also added running as a pre-commit hook with the following code:
require 'english'
require 'rubocop'
ADDED_OR_MODIFIED = /A|AM|^M/.freeze
changed_files = `git status --porcelain`.split(/\n/).
select { |file_name_with_status|
file_name_with_status =~ ADDED_OR_MODIFIED
}.
map { |file_name_with_status|
file_name_with_status.split(' ')[1]
}.
select { |file_name|
File.extname(file_name) == '.rb'
}.join(' ')
system("rubocop #{changed_files}") unless changed_files.empty?
exit $CHILD_STATUS.exitstatus
:clap: :clap: Nice @dimkl!!
One thing I would suggest is to remove the pre-commit hook and replace it with:
- a script that runs the rubocop checks so that we can run it manually and
- a CI step which errors out if rubocop checks are failing (I guess this is done)
The reason I'm suggesting we remove the pre-commit hook is that we should be allowed to commit changes without necessarily conforming to the linter in our local setup. Sometimes you have work in progress that you'd like to commit and the pre-commit hooks get in your way.
The goal would be to ensure that the code that gets merged does not violate the linter rules. What do you think?
- a script that runs the rubocop checks so that we can run it manually and
I would suggest we keep the pre-commit hook as the norm would be to be compliant with our linter and adjust it to match the styles that the teams wants to use. If someone wants to by-pass the pre-commit hook they can use git commit --no-verify .... to bypass the hook.
2.a CI step which errors out if rubocop checks are failing (I guess this is done)
I have already added it, that's why this PR fails. I have added some TODOs in the rubocop file to loosen up the lining rules for now but i will try to resolve them as part of this PR and remove the TODOs. I have added it as a step in the main GH action instead of a different check. cc: @gkats
git commit --no-verify
That works for me! :smile: :tada: