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: Range-color-profile-name.js not using the correct profile name #108

Merged
merged 1 commit into from
Dec 8, 2023

Conversation

Trainermitch
Copy link
Contributor

@Trainermitch Trainermitch commented Dec 7, 2023

Range-color-profile-name.js was using a map iterator object as the profile name instead of the actual string name. Changed the input from a map to an array. Also added a check to make range profile names non case sensitive.

  • I have thoroughly tested all of the code I have modified/added/removed to ensure something else did not break
  • I have followed semantic commit messages e.g. feat: Add foo, chore: Update bar, etc...
  • My branch has a clear history of changes that can be easy to follow when being reviewed commit-by-commit
  • My branch is functionally complete; the only changes to be done will be those potentially requested in code review
  • All changes requested in review have been fixuped into my original commits.
  • Fully tokenized all my strings (no hardcoded English strings!!) and supplied bulk JSON strings below

@tsa96
Copy link
Member

tsa96 commented Dec 7, 2023

Hi, thanks for the PR!

CI is currently failing due to formatting issues, I suggest running npm install and npm run format:fix in the root directory (as detailed in the repo readme), then please amend/fixup that change into your original commit and force-push.

I'll review properly in next few hours!

Copy link
Member

@tsa96 tsa96 left a comment

Choose a reason for hiding this comment

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

Yeah good catch, these fixes seem correct. I'm afraid Prettier wants to do some silly line breaking though, will need to run formatter to get CI to pass (I know, auto formatting stuff can be annoying to faff with, but well worth it for sake of clean code).

@Trainermitch
Copy link
Contributor Author

I reformatted, did that work? This is my first time using github.

@tsa96
Copy link
Member

tsa96 commented Dec 8, 2023

Yup, all good! Congrats on your first PR!

@tsa96 tsa96 merged commit 8a3e448 into momentum-mod:master Dec 8, 2023
1 check passed
@Trainermitch Trainermitch deleted the issue-1 branch December 8, 2023 21:45
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.

2 participants