Add incremental validation #186
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@diareuse could you please file an issue describing the problem you're solving, examples of how and when the new validation mode should be used, as well as listing alternatives and shortcomings of the proposed approach? |
|
Will do |
|
@fzhinkin can we have a followup on this? :) Did you get a second opinion on this feature perhaps? |
Validation is modified when using `useIncrementalValidation=true` to not fail when encountering additions, which are known to be backwards compatible. Instead spits out warnings to allow developers commiting the file early with the changes they have made. The use case is to pair it with CI-run :check (:apiDump) task and commit updated API after merge - considering all steps are automated. In cases where the API is not compatible, the task fails similarly to `useIncrementalValidation=false` or simply _the default_. It's acknowledged that using `useIncrementalValidation=true` might cause issues when building upon a feature (or PR) in which cases the .api file might be left out in its previous state, not commited to the VCS. In which case the developer might prefer leaving the option in the default state (false). To the naked eye it might be apparent that checking only removals (leading `-`) is naive, but since the api diff add two distinct lines for changes (one leading `-`, other `+`), this naive approach proves working for all considered use-cases.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Validation is modified when using
useIncrementalValidation=trueto not fail when encountering additions, which are known to be backwards compatible. Instead spits out warnings to allow developers commiting the file early with the changes they have made.The use case is to pair it with CI-run :check task and commit updated API after merge - considering all steps are automated. In cases where the API is not compatible, the task fails similarly to
useIncrementalValidation=falseor simply the default.It's acknowledged that using
useIncrementalValidation=truemight cause issues when building upon a feature (or PR) in which cases the .api file might be accidentally prematurely commited to the VCS. In which case the developer might prefer leaving the option in the default state.To the naked eye it might be apparent that checking only removals (leading
-) is naive, but since the api diff add two distinct lines for changes (one leading-, other+), this naive approach proves working for all considered use-cases.Change also includes ignored test, which ensures that
apiDumpis run automatically. I was not able, unfortunately, to get this feature working due to time constraints and not wanting to introduce additional Gradle tasks to automate this task. Maintainers are advised to either delete the test or implement the feature at their own pace. 🙂