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 check command #231

Merged
merged 10 commits into from
Mar 15, 2021
Merged

Add check command #231

merged 10 commits into from
Mar 15, 2021

Conversation

mutecipher
Copy link
Contributor

@mutecipher mutecipher commented Feb 17, 2021

Motivation

We need an easy way to check whether or not the RBI files are out-of-date after updates are made to files, Gems are bumped, etc. This adds a check command to tapioca that will re-generate the DSL (excluding Gems) to diff against to determine whether or not tapioca dsl needs to be run.

Implementation

Create a tmpdir using Dir.mktmpdir, generate the DSL in the temp directory, and diff against that to determine if RBIs are out of sync.

The command returns with an exit status of 0 if no changes are detected, and 1 if changes are detected. This will allow for it to be used in CI pipelines.

Tests

Adds two additional tests that ensure that the command returns the correct exit code as well as messaging to advise on correct next steps.

@mutecipher mutecipher self-assigned this Feb 17, 2021
@mutecipher mutecipher changed the title add check command Add check command Feb 17, 2021
@mutecipher mutecipher force-pushed the add-check-command branch 6 times, most recently from c73d648 to c049deb Compare February 22, 2021 22:11
@mutecipher mutecipher force-pushed the add-check-command branch 11 times, most recently from 73cd8d5 to b794ebb Compare March 3, 2021 21:28
@mutecipher mutecipher force-pushed the add-check-command branch 3 times, most recently from 68f8a0b to 0ac3d8e Compare March 9, 2021 18:54
@mutecipher mutecipher requested a review from a team March 11, 2021 19:10
@mutecipher mutecipher marked this pull request as ready for review March 11, 2021 19:10
spec/tapioca/cli_spec.rb Outdated Show resolved Hide resolved
spec/tapioca/cli_spec.rb Outdated Show resolved Hide resolved
lib/tapioca/generator.rb Outdated Show resolved Hide resolved
lib/tapioca/generator.rb Outdated Show resolved Hide resolved
lib/tapioca/generator.rb Outdated Show resolved Hide resolved
lib/tapioca/generator.rb Outdated Show resolved Hide resolved
RBI files are out-of-date, please run `generate command` to update.
Reason: New file(s) introduced.
OUTPUT
assert_includes($?.to_s, "exit 1") # rubocop:disable Style/SpecialGlobalVars
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Want to make a special note about this. I was intending to use Process.last_status which wasn't available in Ruby 2.4. So I opted to use this with the rubocop exception as it's just for tests to work around the IO.popen(...)

lib/tapioca/cli.rb Outdated Show resolved Hide resolved
lib/tapioca/cli.rb Outdated Show resolved Hide resolved
lib/tapioca/generator.rb Show resolved Hide resolved
lib/tapioca/generator.rb Outdated Show resolved Hide resolved
spec/tapioca/cli_spec.rb Show resolved Hide resolved
spec/tapioca/cli_spec.rb Outdated Show resolved Hide resolved
lib/tapioca/cli.rb Outdated Show resolved Hide resolved
lib/tapioca/generator.rb Outdated Show resolved Hide resolved
lib/tapioca/generator.rb Show resolved Hide resolved
lib/tapioca/config_builder.rb Outdated Show resolved Hide resolved
lib/tapioca/generator.rb Show resolved Hide resolved
lib/tapioca/generator.rb Outdated Show resolved Hide resolved
lib/tapioca/generator.rb Outdated Show resolved Hide resolved
lib/tapioca/generator.rb Show resolved Hide resolved
spec/tapioca/cli_spec.rb Show resolved Hide resolved
Copy link
Contributor

@RyanBrushett RyanBrushett left a comment

Choose a reason for hiding this comment

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

🎩 seems to work neatly to me!
Let's cut an issue for the refactors somewhere IMO since there are a few worth tracking.
Possible future improvement: Quieter output that just writes the result and the reason like

RBI files are out-of-date, please run `tapioca dsl` to update.
Reason: New file(s) introduced.

option :verify,
type: :boolean,
default: false,
desc: "Verifies RBIs are up-to-date"
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be worth explicitly saying that it doesn't modify anything in the description.

Copy link
Member

@vinistock vinistock left a comment

Choose a reason for hiding this comment

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

🚀

Copy link
Contributor

@KaanOzkan KaanOzkan left a comment

Choose a reason for hiding this comment

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

Thanks Cory, nice job 🙂

@mutecipher mutecipher merged commit 30ad93d into master Mar 15, 2021
@mutecipher mutecipher deleted the add-check-command branch March 15, 2021 17:16
@mutecipher mutecipher temporarily deployed to production March 22, 2021 20:31 Inactive
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

6 participants