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

style(senna): fix linting errors #702

Merged
merged 5 commits into from
Oct 20, 2021
Merged

style(senna): fix linting errors #702

merged 5 commits into from
Oct 20, 2021

Conversation

izaera
Copy link
Member

@izaera izaera commented Oct 19, 2021

Just that!

@github-actions github-actions bot added the senna label Oct 19, 2021
Copy link
Contributor

@diegonvs diegonvs left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@bryceosterhaus bryceosterhaus left a comment

Choose a reason for hiding this comment

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

Just had one part I think we need to address, but the others look good.

I'm impressed you went through the trouble of updating all the should in tests. I would've just turned off that rule inside of senna...

maintenance/projects/senna/src/screen/RequestScreen.js Outdated Show resolved Hide resolved
@izaera
Copy link
Member Author

izaera commented Oct 20, 2021

I'm impressed you went through the trouble of updating all the should in tests.

😂 I made a vim macro for that: remove should, move one word right, add an s. That made it for 90% of the tests since English behaves as a good language in this case.

Otherwise I would have changed eslint config, most probably 🙄

One funny thing I noticed is that someone, at some moment in time, had to add some shoulds, because of how they were written in some of the tests (incorrectly) 🎉

@izaera
Copy link
Member Author

izaera commented Oct 20, 2021

Ready to go 🚀

I had to forcepush to avoid a "fabricated" fix semantic commit that would have appeared in the changelog, maybe.

@izaera
Copy link
Member Author

izaera commented Oct 20, 2021

I'll merge this since I've fixed this issue and the rest was approved.

@izaera izaera merged commit f6a283e into liferay:master Oct 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants