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

Docs: Propose fix typo for function #9965

Merged
merged 1 commit into from Feb 9, 2018
Merged

Conversation

jeis2497052
Copy link
Contributor

What is the purpose of this pull request? (put an "X" next to item)

[ X] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofixing to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:

What changes did you make? (Give an overview)

Is there anything you'd like reviewers to focus on?

@jsf-clabot
Copy link

jsf-clabot commented Feb 9, 2018

CLA assistant check
All committers have signed the CLA.

@eslint-deprecated eslint-deprecated bot added the triage An ESLint team member will look at this issue soon label Feb 9, 2018
@abiduzz420
Copy link
Contributor

abiduzz420 commented Feb 9, 2018

Hello @jeis2497052
You need to follow the commit-message guidelines. The check failed because the commit message did not start with label Docs: . Also edit the PR title. Example: Docs: Propose fix typo for function

@jeis2497052
Copy link
Contributor Author

Sorry, can I repair this PR or start again?

@jeis2497052
Copy link
Contributor Author

Not sure if or how to repair?

@abiduzz420
Copy link
Contributor

abiduzz420 commented Feb 9, 2018

PR title could be directly edited from the UI. For making changes to the commit message, you could take the help of this: https://help.github.com/articles/changing-a-commit-message/

EDIT: If you need additional help, here it is

1. Type git commit --amend
2. Enter the commit message: Docs: Propose fix typo for function
3. git push --force origin master

@jeis2497052 jeis2497052 changed the title Propose fix typo for function Doc: Propose fix typo for function Feb 9, 2018
@jeis2497052 jeis2497052 changed the title Doc: Propose fix typo for function Docs: Propose fix typo for function Feb 9, 2018
@jeis2497052
Copy link
Contributor Author

is the UI edit OK now with Docs: added?

@platinumazure
Copy link
Member

Hi @jeis2497052, thanks for contributing!

Our commit message check will look at either the first commit's commit message, or the PR title, depending on the number of commits in the PR. If the PR has 1 commit, we look at the commit message; if 2 or more, we look at the PR title. This is because GitHub's squash and merge interface uses the exact same logic to decide what commit message to suggest when we are about to merge.

Right now, your PR title matches our guidelines, but the commit message does not, and there is only one commit.

I think we could handle this using one of the following approaches:

  1. Ignore the issue, and we'll make sure the commit message matches the PR title on merge
  2. You could find another thing to change (ideally in the same file) and push another commit, which will then make our commit message check look at the PR title
  3. You could use git commit --amend, then git push (remote-name) (branch-name) -f, as suggested by @abiduzz420.

The last option is probably the "best" option, but also the most annoying on your end. Let me know how you would like to proceed.

@platinumazure platinumazure added documentation Relates to ESLint's documentation accepted There is consensus among the team that this change meets the criteria for inclusion core Relates to ESLint's core APIs and features and removed triage An ESLint team member will look at this issue soon labels Feb 9, 2018
@jeis2497052
Copy link
Contributor Author

Did I properly amend ?

@platinumazure
Copy link
Member

@jeis2497052 Yes! Everything looks great now and our commit-message status check is happy 😄 Thanks so much for taking the time to do this, it really helps us out and it makes our changelog beautiful.

TravisCI and AppVeyor need to rerun due to the forced push, but hopefully those will pass soon enough.

@ilyavolodin
Copy link
Member

Thanks for the pull request.

This was referenced Mar 19, 2018
@@ -20,7 +20,7 @@ Examples of **incorrect** code for this rule:
typeof foo === "strnig"
typeof foo == "undefimed"
typeof bar != "nunber"
typeof bar !== "fucntion"
typeof bar !== "function"
Copy link
Contributor

Choose a reason for hiding this comment

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

@platinumazure @jeis2497052 I think this is wrong. These are deliberate typos, aren't they?

Copy link
Member

Choose a reason for hiding this comment

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

Agreed, would you like to create a PR to fix this example?

Copy link
Contributor

Choose a reason for hiding this comment

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

#10459 :)

Wilfred added a commit to Wilfred/eslint that referenced this pull request Jun 9, 2018
This was accidentally removed in eslint#9965.
Wilfred added a commit to Wilfred/eslint that referenced this pull request Jun 9, 2018
This was accidentally removed in eslint#9965.
platinumazure pushed a commit that referenced this pull request Jun 9, 2018
@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Aug 9, 2018
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Aug 9, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion core Relates to ESLint's core APIs and features documentation Relates to ESLint's documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants