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

service-name warn only once if there is an oldBehavior=true #432

Merged
merged 2 commits into from
Mar 5, 2017

Conversation

oriSomething
Copy link
Collaborator

@oriSomething oriSomething commented Jan 1, 2017

Related to #431

Also when there was no oldBehavior at all since it's default to true

  • Test locally

@coveralls
Copy link

coveralls commented Jan 1, 2017

Coverage Status

Coverage decreased (-0.07%) to 99.927% when pulling ea79123 on service-name-fix into a6fa1a6 on master.

@EmmanuelDemey
Copy link
Owner

Any update about this PR ? and about my comment ?

@oriSomething
Copy link
Collaborator Author

Sorry, barely have the time, since need to fix the 100% coverage.
If i understood your comment, I've moved it to before the return of the Object for ESLint traversing

@EmmanuelDemey
Copy link
Owner

But do we need this didWarnForDeprecatedBehavior flag ?

@oriSomething
Copy link
Collaborator Author

It's only made for logging the warning only once, and to flood like described in the issue, since the context param might changed by different files

@oriSomething
Copy link
Collaborator Author

I thought again, If oldBehavior is undefined, Maybe we should use ESLint warning instead of console.log? It won't break parse, and UX for the programmer will be better

@coveralls
Copy link

coveralls commented Feb 5, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling d56afa6 on service-name-fix into a6fa1a6 on master.

@oriSomething
Copy link
Collaborator Author

@Gillespie59 This PR is ready, but maybe I should replace the console.warn with ESLint warning?

@EmmanuelDemey
Copy link
Owner

But I do not the syntax for ESLint warnings :s

@oriSomething
Copy link
Collaborator Author

@Gillespie59 Sorry, I didn't understand

@EmmanuelDemey
Copy link
Owner

With the complete sentence ... sorry
But I do not know the syntax for ESLint warnings :s. What do you meen by ESLint warning ?

@oriSomething
Copy link
Collaborator Author

Ah, I meant by reporting ESLing warning (as reporting error) like any ESLint errors.
Reporting about "deprecated syntax"

@mateatslc
Copy link

would be nice to see this move

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants