JS: refactor get predicates with no return #18187
Draft
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
asgerf
reviewed
Dec 3, 2024
| /** Holds if this package depends on version 'version' of package 'pkg'. */ | ||
| predicate getADependency(string pkg, string version) { version = this.getPropStringValue(pkg) } | ||
| /** Returns the version of the specified package that this package depends on. */ | ||
| string getADependency(string pkg) { result = this.getPropStringValue(pkg) } |
There was a problem hiding this comment.
I know this is in draft PR state, but I a peek anyway.
We have to be wary of breaking changes in predicates that are part of the public API. To change the signature of this predicate, we'd have to rename it and leave behind a deprecated version of the original predicate.
Apart from that, I think I'd prefer to rename it to hasDependency instead, and not add a return value.
There was a problem hiding this comment.
And if we rename it to hasDependency, we would still need to keep the original predicate getADependency marked as deprecated and in the comment refer that people now should use hasDependency, correct?
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.
This small PR fixes an issue in the
JavaScriptside ofQLwhere predicates starting withgetorasdon't have a return type.The query introduced from #18164 helped identify these predicates.