-
Notifications
You must be signed in to change notification settings - Fork 78
Change 'test cases' to 'examples' as required by ACT Rules Format 1.1 #2352
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
base: develop
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for act-rules ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Co-authored-by: Kathy Eng <kengdoj@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left some suggestions. Some are changes to path which I'm not sure of so please review.
Co-authored-by: Kathy Eng <kengdoj@users.noreply.github.com>
Co-authored-by: Kathy Eng <kengdoj@users.noreply.github.com>
Co-authored-by: Kathy Eng <kengdoj@users.noreply.github.com>
Co-authored-by: Kathy Eng <kengdoj@users.noreply.github.com>
Co-authored-by: Kathy Eng <kengdoj@users.noreply.github.com>
Co-authored-by: Kathy Eng <kengdoj@users.noreply.github.com>
Co-authored-by: Kathy Eng <kengdoj@users.noreply.github.com>
Co-authored-by: Kathy Eng <kengdoj@users.noreply.github.com>
Co-authored-by: Kathy Eng <kengdoj@users.noreply.github.com>
Co-authored-by: Kathy Eng <kengdoj@users.noreply.github.com>
Thanks for picking these ones up @kengdoj
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These changes look good to me, but I think we probably should do at least a 1-week call for review for this.
The review period has been updated.
/** | ||
* Check if filename has `id` as a part of the name | ||
*/ | ||
test('each testcase has a heading', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are we also changing testcase
(with no space) to example?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@daniel-montalvo can you look at this. Discussed on the call, we can do this in a separate PR too if you prefer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just one non blocking comment
The format now says that the test cases should be named examples, and it has a MUST on it, so we should do this before attempting to transition to REC.
Closes issue(s):
Need for Call for Review:
This will require a 1-week call for review (changes the name of one of the key sections of our rules).
Pull Request Etiquette
When creating PR:
develop
branch (left side).After creating PR:
Rule
,Definition
orChore
.When merging a PR:
How to Review And Approve