Skip to content
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

Add a case to the warning #2965

Closed
wants to merge 1 commit into from
Closed

Add a case to the warning #2965

wants to merge 1 commit into from

Conversation

keePeek
Copy link
Contributor

@keePeek keePeek commented Mar 9, 2021

Checklist — To help your pull request get merged faster, please do following:

    • Provide a summary of your changes — Add a case to the "warning" section in the page.
    • Provide a link to the issue(s) you are fixing, if appropriate, in the form "Fixes url-of-issue". GitHub will render this in the form "Fixes fix: fixable flaws in Glossary A-D entries #1234", with the issue number linked to the issue. Doing this allows us to figure out what issues you are fixing, as well as helping to automate things (for example the issue will be closed once the PR that fixed it has been merged).
    • Review the results of the automated checking we run on every PR and fix any problems reported (see the list of checks near the bottom of the PR page). If you need help, please ask in a comment!
    • Link to any other resources that you think might be useful in reviewing your PR.

@keePeek keePeek requested a review from a team as a code owner March 9, 2021 06:34
@keePeek keePeek requested review from Rumyra and removed request for a team March 9, 2021 06:34
@chrisdavidmills
Copy link
Contributor

@keePeek can you please update the description to provide a summary of your changes? You've ticked the checklist box, but you've not added a description of what you are doing here and why. Thanks!

@keePeek
Copy link
Contributor Author

keePeek commented Mar 18, 2021

@keePeek can you please update the description to provide a summary of your changes? You've ticked the checklist box, but you've not added a description of what you are doing here and why. Thanks!

Obviously the first input box (<input id="commit-summary-input">) before the submit button is the summary to be filled in (also automatically converted to the title of this issue), where do you want me to write it again?

Why is it so troublesome after MDN switched to GitHub? I used to directly contribute and edit a few pages on the MDN website, and it was quite simple and easy at that time, just like Wikipedia or StackOverflow, but now it is more cumbersome (at least five more steps) to contribute to MDN!
This makes contributors very frustrated, please simplify and improve the submission process!

@chrisdavidmills
Copy link
Contributor

@keePeek can you please update the description to provide a summary of your changes? You've ticked the checklist box, but you've not added a description of what you are doing here and why. Thanks!

Obviously the first input box (<input id="commit-summary-input">) before the submit button is the summary to be filled in (also automatically converted to the title of this issue), where do you want me to write it again?

I got confused because you used the word "case" in your title, so it made me think your addition was supposed to be something about string casing. But I guess you mean it in a more general sense.

Why is it so troublesome after MDN switched to GitHub? I used to directly contribute and edit a few pages on the MDN website, and it was quite simple and easy at that time, just like Wikipedia or StackOverflow, but now it is more cumbersome (at least five more steps) to contribute to MDN!
This makes contributors very frustrated, please simplify and improve the submission process!

I can underatand your frustration. It is always a pain to get used to a new workflow. But for simple changes I don't think it is any more complex than the old Wiki system. You are able to just go to the source page in GitHub, press the little "Edit this file" button, make your changes, and then submit a PR right from the Web UI.

For larger changes, it is better to set up the local Yari installation as detailed in the README.

And there are so many advantages to the new system, like proper reviewing, mass edits, community communication, and more.

@keePeek
Copy link
Contributor Author

keePeek commented Mar 18, 2021

So, is there anything else I need to do? Is there still something unreasonable in my submission?

@chrisdavidmills
Copy link
Contributor

So, is there anything else I need to do? Is there still something unreasonable in my submission?

Your submission hasn't been reviewd yet. There reviewer has been set to @Rumyra , and she will get to it soon, I'm sure.

Copy link
Collaborator

@Rumyra Rumyra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great @keePeek thank you! Just a minor update to include a link to lastIndex 👍

@Rumyra
Copy link
Collaborator

Rumyra commented Mar 30, 2021

Hi @keePeek if you commit the above suggestions we should be able to get this merged - thank you ☺️

Copy link
Contributor Author

@keePeek keePeek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Rumyra Please review and merge the revised latest full text I attached below (replace all the source code I submitted before).

@Ryuno-Ki
Copy link
Collaborator

@Rumyra Ping

@Rumyra
Copy link
Collaborator

Rumyra commented Apr 22, 2021

Hi @keePeek this looks great, if you can add it to this PR that would also be great - thank you 🙏

Comment on lines +187 to +189

<p>In addition, when matching zero-length characters (e.g. <code>/^/gm</code>),
<code>++regexp.lastIndex</code> can be used to avoid infinite loops.</p>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<p>In addition, when matching zero-length characters (e.g. <code>/^/gm</code>),
<code>++regexp.lastIndex</code> can be used to avoid infinite loops.</p>
<p>In addition, when matching zero-length characters (e.g. <code>/^/gm</code>),
increase its {{jsxref("RegExp.lastIndex", "lastIndex")}} each time to avoid
an infinite loop.</p>

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This modification is of course the final version! 🙏 Plz merge, thx!~

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I just realized that the previous collaboration failure should be blamed on me for not knowing the meaning of "Resolve conversation", I shouldn't click this button. Sorry everyone (with embarrassment)!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sideshowbarker
Copy link
Collaborator

Closing for lack of response from OP

@keePeek

This comment has been minimized.

@Rumyra
Copy link
Collaborator

Rumyra commented Jul 12, 2021

Hi @keePeek you should see a 'Commit Suggestion' button when viewing this issue (only the PR owner can see it) If you click that we should be able to get this merged 👍

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 31, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants