-
Notifications
You must be signed in to change notification settings - Fork 34.7k
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
feat(contains): create exercise #442
base: main
Are you sure you want to change the base?
feat(contains): create exercise #442
Conversation
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 imagine you're leaving out contains.spec.js
to make it easier to do changes to only one file instead of two, but just a reminder in case that wasn't your intention and you just forgot!
Co-authored-by: MaoShizhong <[email protected]>
…avascript-exercises into feat/exercise_14
@NikitaRevenco to make things easier to know when to do things, once you've made any requested changes etc., if you could re-request a review from whichever maintainer, that would make it much easier to know when it's okay to review (so we don't go to review and submit comments right before you push another commit that changes things again) |
Alright, that makes sense! |
I've opened the next exercise for review! I wanted to 'request' - but seems like that cannot be done on a PR with no reviews yet. For the future, do you prefer me to add a comment when the next PR is ready for review or? |
@NikitaRevenco No need to comment or request a review when un-drafting something. The un-drafting itself is enough to signify that something is ready for review. I'm not reviewing these because I'm assigned them, nor am I the only one allowed to review them. I just happen to be seeing these and choosing to add my reviews. Any of the maintainers may wish to review (or not) at any time, and it wouldn't be unrealistic if they had different opinions to me or comments on things I hadn't considered. And I'd probably be wanting at least another opinion before they're all finalised for merging. |
Co-authored-by: MaoShizhong <[email protected]>
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.
A few more comments.
Co-authored-by: MaoShizhong <[email protected]>
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.
@JoshDevHub Any further thoughts yourself?
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.
Looks good I think 👍
Co-authored-by: MaoShizhong <[email protected]>
Because
It was decided to add new recursion exercises
Previous
This PR
Issue
Related to #27265
Additional Information
Pull Request Requirements
location of change: brief description of change
format, e.g.01_helloWorld: Update test cases
Because
section summarizes the reason for this PRThis PR
section has a bullet point list describing the changes in this PRIssue
section/solutions
folder