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

Fix 210: New rule for behavior vs behaviour. #212

Merged
merged 5 commits into from
Oct 6, 2021

Conversation

jackyhui96
Copy link
Contributor

No description provided.

Copy link
Member

@elbrujohalcon elbrujohalcon left a comment

Choose a reason for hiding this comment

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

Thank you very much, @jackyhui96 !!
Two minimal adjustments and it will be ready to be merged :)

src/elvis_style.erl Outdated Show resolved Hide resolved
src/elvis_style.erl Outdated Show resolved Hide resolved
src/elvis_style.erl Outdated Show resolved Hide resolved
@jackyhui96
Copy link
Contributor Author

jackyhui96 commented Oct 4, 2021

Will work on the changes now and push up, @elbrujohalcon could you label it with "hacktoberfest-accepted" so it counts towards my hacktoberfest PRs? :) Was meant to add this to your original issue which has the label but not sure on the correct process to do that

Copy link
Member

@elbrujohalcon elbrujohalcon left a comment

Choose a reason for hiding this comment

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

I updated the repo tags (It had hacktober instead of hacktoberfest 🤦🏻 ).
I left a few suggestions to improve the code, too.
Besides them, now that we no longer support american/british as spelling types, I would be tempted to rename spelling_type as proper_spelling, or just spelling. What do you think?

src/elvis_style.erl Outdated Show resolved Hide resolved
src/elvis_style.erl Outdated Show resolved Hide resolved
src/elvis_style.erl Outdated Show resolved Hide resolved
Copy link
Member

@elbrujohalcon elbrujohalcon left a comment

Choose a reason for hiding this comment

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

This is great, @jackyhui96! Thank you very much!
The only missing things are to add the new rule to the list of default rules for erl_files and beam_files rulesets and to update the wiki.

@jackyhui96
Copy link
Contributor Author

This is great, @jackyhui96! Thank you very much! The only missing things are to add the new rule to the list of default rules for erl_files and beam_files rulesets and to update the wiki.

No worries @elbrujohalcon, happy to help and contribute! I've updated the the wiki and pushed the default rule changes now

@elbrujohalcon
Copy link
Member

Woah! Now we found other tests are broken… 🤦🏻

@elbrujohalcon
Copy link
Member

…aaaaaand fixed!! 🪄

@elbrujohalcon elbrujohalcon merged commit 8388fca into inaka:master Oct 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants