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

Chore: use meta.messages in some rules (2/4) (refs #9870) #10773

Merged
merged 67 commits into from Jan 18, 2019

Conversation

aladdin-add
Copy link
Member

@aladdin-add aladdin-add commented Aug 18, 2018

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

[ ] 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
[x] Other, please explain:

What changes did you make? (Give an overview)
use messages in some rules, to help to support the localization of error messages.

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

I plan to separate it to 4 PRs. temporally disabled the rule, and will enable it when all the changes finished.

@aladdin-add aladdin-add added accepted There is consensus among the team that this change meets the criteria for inclusion evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion chore This change is not user-facing labels Aug 18, 2018
@aladdin-add aladdin-add changed the title Chore: use meta.messages in some rules (2/4) Chore: use meta.messages in some rules (2/4) Aug 18, 2018
@aladdin-add aladdin-add changed the title Chore: use meta.messages in some rules (2/4) wip:Chore: use meta.messages in some rules (2/4) Aug 18, 2018
@aladdin-add aladdin-add force-pushed the chore/messages branch 3 times, most recently from f8c5e02 to e993ecb Compare August 18, 2018 20:31
@not-an-aardvark
Copy link
Member

not-an-aardvark commented Aug 19, 2018

Closing/reopening to see if the Appveyor build is fixed. (I think I temporarily broke it by enabling OAuth App access restrictions.)

edit: It doesn't seem like it worked on this PR, which is strange because it definitely should have access now. Maybe it will work for new PRs.

@not-an-aardvark
Copy link
Member

not-an-aardvark commented Aug 19, 2018

Trying again, it seems like Appveyor has two separate OAuth apps ("Appveyor" and "Appveyor CI").

edit: Seems like it worked this time 🎉

@aladdin-add aladdin-add force-pushed the chore/messages branch 5 times, most recently from 6e120be to e974343 Compare August 19, 2018 19:02
@aladdin-add aladdin-add removed the evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion label Aug 20, 2018
@aladdin-add aladdin-add force-pushed the chore/messages branch 2 times, most recently from fbd5867 to 33b849b Compare August 24, 2018 10:23
@aladdin-add aladdin-add force-pushed the chore/messages branch 3 times, most recently from 7cb8bca to 26e7392 Compare August 30, 2018 18:50
@platinumazure
Copy link
Member

Hi @aladdin-add, what are the next steps here?

As an aside, would it be possible to edit the initial post to fill out the PR template? That way we can get some context when reviewing. 😄 Thanks!

@platinumazure platinumazure added the do not merge This pull request should not be merged yet label Sep 14, 2018
@aladdin-add aladdin-add changed the title wip:Chore: use meta.messages in some rules (2/4) Chore: use meta.messages in some rules (2/4) Sep 30, 2018
@aladdin-add
Copy link
Member Author

I've updated and rebased, could you please take another look at the commit 5780538 ? 😄

@kaicataldo kaicataldo merged commit eb5c401 into eslint:master Jan 18, 2019
@kaicataldo
Copy link
Member

Thanks for contributing to ESLint!

@aladdin-add aladdin-add deleted the chore/messages branch January 19, 2019 13:27
@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Jul 18, 2019
@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 Jul 18, 2019
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 chore This change is not user-facing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants