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 "force" option for compare-file-text #251

Merged
merged 1 commit into from
May 6, 2021

Conversation

kpaulisse
Copy link
Contributor

@kpaulisse kpaulisse commented May 5, 2021

Overview

This pull request modifies the --compare-file-text option, which was previously just a binary option, to accept an additional parameter. There are now 3 behaviors:

  • --compare-file-text -- Like before, enables the feature
  • --no-compare-file-text -- Like before, disables the feature
  • --compare-file-text=force -- New! Enables the feature, and doesn't let it be auto-disabled

We need this behavior because we're using --catalog-only in our workflow, and this currently auto-disables the feature because the second catalog in the non-existent diff is a "NoOp" catalog. With this change, our generated catalog will be populated with the content of static files rather than a reference to them, which is what we want.

I chose this implementation because it preserves existing behavior while still not introducing yet another command line argument. I added an integration test for this as well.

Checklist

  • Make sure that all of the tests pass, and fix any that don't. Just run rake in your checkout directory, or review the CI job triggered whenever you push to a pull request.
  • Make sure that there is 100% test coverage by running rake coverage:spec or ignoring untestable sections of code with # :nocov comments. If you need help getting to 100% coverage please ask; however, don't just submit code with no tests.
  • If you have added a new command line option, we would greatly appreciate a corresponding integration test that exercises it from start to finish. This is optional but recommended.

@kpaulisse kpaulisse marked this pull request as ready for review May 5, 2021 21:51
@ahayworth ahayworth added this to the 2.2.0 milestone May 6, 2021
Copy link
Contributor

@ahayworth ahayworth left a comment

Choose a reason for hiding this comment

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

Seems alright to me - like you mentioned in the docs: if you know what you're doing and want to force it, then we can let you! 😄

@ahayworth ahayworth merged commit d2a25a3 into github:master May 6, 2021
@kpaulisse kpaulisse mentioned this pull request May 11, 2021
3 tasks
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.

2 participants