Refactor: extract function for canonicalising modeled methods#3518
Merged
shati-patel merged 1 commit intomainfrom Mar 27, 2024
Merged
Refactor: extract function for canonicalising modeled methods#3518shati-patel merged 1 commit intomainfrom
shati-patel merged 1 commit intomainfrom
Conversation
We don't currently (I think) have an easy way to check whether two modeled methods are the same. I've shuffled some stuff around, so that you can now compare modeled methods by check that their "key" (i.e. JSON-stringified form) is the same.
charisk
approved these changes
Mar 27, 2024
Contributor
charisk
left a comment
charisk
left a comment
There was a problem hiding this comment.
LGTM, but since we paired, I'd be keen to get reviews from others too.
koesie10
approved these changes
Mar 27, 2024
Member
koesie10
left a comment
koesie10
left a comment
There was a problem hiding this comment.
LGTM, but it would be nice to add some tests. This method was implicitly tested by validation.test.ts before, but now that it has been moved around and is in a different file, it might be good to add some tests for this method specifically.
Contributor
Author
|
Good idea about unit tests! I'll do that in a follow-up PR 👀 |
3 tasks
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
[🍐 paired with Charis]
We don't currently (I think) have an easy way to check whether two modeled methods are the same. I've shuffled some stuff around, so that you can now compare modeled methods by checking that their "key" is the same, i.e.
createModeledMethodKey(method1) === createModeledMethodKey(method2).(Not used yet, but I plan to use this for jumping between corresponding models in the model editor and the model alerts view)
Checklist
N/A: no user-facing changes
ready-for-doc-reviewlabel there.