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

Support eslint fix command in fixable rules #202

Open
6 tasks
renatoagds opened this issue Jul 27, 2020 · 8 comments
Open
6 tasks

Support eslint fix command in fixable rules #202

renatoagds opened this issue Jul 27, 2020 · 8 comments
Labels
enhancement New feature or request pinned Pinned for different reasons. Issues with this label won't be flagged as stale by stalebot

Comments

@renatoagds
Copy link
Contributor

renatoagds commented Jul 27, 2020

As suggested in #186 we could start to allow fix for rules that's possible to do that.

Rules that we could implement fix:

I'm working to map more rules that's open for that

@Belco90
Copy link
Member

Belco90 commented Jul 27, 2020

Are we sure we can implement the fix code for those with ease? They seem quite complex or even ambiguous. How do you plan to fix both of them?

I was thinking about adding fix code for some other rules that are more simple than those, like await-async-query, await-async-utils, await-fire-eventor no-await-sync-query.

@Belco90 Belco90 added the enhancement New feature or request label Jul 27, 2020
@renatoagds
Copy link
Contributor Author

How do you plan to fix both of them?

  • In no-side-effects-wait-for I was thinking about moving only side effects before waitFor, since they probably do some action that's expected in waitFor
  • In no-multiple-assertions-wait-for I was thinking about keep the first expect and move the others after waitFor.

Makes sense?

I was thinking about adding fix code for some other rules that are more simple than those, like await-async-query, await-async-utils, await-fire-event or no-await-sync-query.

Good, will add them in the current list here

@Belco90
Copy link
Member

Belco90 commented Jul 27, 2020

Those fix strategies sound fine! Let us know if you find any edge case when implementing any of them.

@renatoagds
Copy link
Contributor Author

Sorry guys .. Have been out for a long time, but will return helping the project, starting for this issue 🤘

@renatoagds
Copy link
Contributor Author

Did we had some change into the scenario exposed here? cc/ @Belco90

@Belco90
Copy link
Member

Belco90 commented Oct 26, 2020

I don't think so, but these rules are gonna be refactor for v4 pretty soon. I don't know if it would be better to wait for v4 then? Not a major issue, but will cause conflicts on v4 of course.

@renatoagds
Copy link
Contributor Author

@Belco90 Makes sense!! That's something more that I could help? Maybe the rules refactor

@Belco90
Copy link
Member

Belco90 commented Oct 26, 2020

I guess we have two options:

  • implement this fixable code within v4 itself, so we implement as much as we can while finishing the v4. Then, we continue with fixable code for remaining rules after v4 release
  • put more effort on v4 rules refactor and after that we go back to this ticket

Ideally I'd prefer not stopping adding/fixing things in current v3, but all modifications necessary for this issue will slow me down in v4 since I have to update v4 branches, adapt the code, check everything works as expected etc. And there would be a lot of conflicts when updating v4 with master.

So I suggest you to read this comment where you can find the analysis for necessary work on v4, and then check the current merged PRs for #198 (you can find them in "linked pull requests" section of that issue) so you get a better understanding about the new structure of the plugin. Then I guess we can organize the work if you could help with v4 (moving tasks from my notion analysis to this repo's Project).

Go through that and let me know what you think! Answer in #198 if you think that's more appropriate.

@Belco90 Belco90 pinned this issue Apr 11, 2021
@Belco90 Belco90 added the pinned Pinned for different reasons. Issues with this label won't be flagged as stale by stalebot label Apr 11, 2021
@Belco90 Belco90 unpinned this issue Jan 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request pinned Pinned for different reasons. Issues with this label won't be flagged as stale by stalebot
Projects
None yet
Development

No branches or pull requests

2 participants