Skip to content

Comments

LED naming update in code#2621

Merged
hoffie merged 1 commit intojamulussoftware:masterfrom
henkdegroot:LED-naming
May 20, 2022
Merged

LED naming update in code#2621
hoffie merged 1 commit intojamulussoftware:masterfrom
henkdegroot:LED-naming

Conversation

@henkdegroot
Copy link
Contributor

@henkdegroot henkdegroot commented Apr 27, 2022 •
edited
Loading

Short description of changes

LED image files and references update to match.

CHANGELOG: Internal: Rename LED resources

Context: Fixes an issue?

Fixes: #2365

Does this change need documentation? What needs to be documented and how?

No

Status of this Pull Request

Complete

What is missing until this pull request can be merged?

Nothing

Checklist

  • I've verified that this Pull Request follows the general code principles
  • I tested my code and it does what I want
  • My code follows the style guide
  • I waited some time after this Pull Request was opened and all GitHub checks completed without errors.
  • I've filled all the content above

@ann0see ann0see self-requested a review April 27, 2022 07:56
ann0see
ann0see reviewed Apr 28, 2022
View reviewed changes
Copy link
Member

@ann0see ann0see left a comment

Choose a reason for hiding this comment

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

Why are some LEDs smaller now? Did you Minify them?

@henkdegroot
Copy link
Contributor Author

henkdegroot commented Apr 29, 2022 •
edited
Loading

The LEDs are still the same, just the filenames have changed which makes it a bit difficult to see.

For instance, the "Big" version of the Round LED was named: CLEDBlackSmall.png and the "Small" version was named: LEDBlackSmall.png. Which is what was kind of confusing.
The new name for the CLEDBlackSmall.png is CLEDBlackBig.png and the new name for the LEDBlackSmall.png is CLEDBlackSmall.png.

Because the filename CLEDBlackSmall.png is the same as the previous "big" version, git shows it as if these are now smaller.
The CLEDBlackBig.png (new filename) is shown as added and the LEDBlackSmall.png is shown as deleted.

Hope this makes sense.

@ann0see
Copy link
Member

ann0see commented Apr 29, 2022

Ok. Thanks.

ann0see
ann0see approved these changes Apr 29, 2022
View reviewed changes
Copy link
Member

@ann0see ann0see left a comment

Choose a reason for hiding this comment

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

Tested and looks good.

@ann0see ann0see requested a review from gilgongo April 29, 2022 20:22
@hoffie hoffie added this to the Release 3.9.0 milestone May 20, 2022
hoffie
hoffie approved these changes May 20, 2022
View reviewed changes
Copy link
Member

@hoffie hoffie left a comment

Choose a reason for hiding this comment

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

Thanks @henkdegroot and sorry for leaving this laying around for so long!
Naming seems good, styles still seem to work properly.

@hoffie hoffie merged commit bc2f246 into jamulussoftware:master May 20, 2022
@henkdegroot henkdegroot deleted the LED-naming branch May 21, 2022 06:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hoffie hoffie hoffie approved these changes

@ann0see ann0see ann0see approved these changes

@gilgongo gilgongo Awaiting requested review from gilgongo

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

Release 3.9.0

Development

Successfully merging this pull request may close these issues.

Levelmeter: Ensure that variables and png names (roughly) match

3 participants

@henkdegroot @ann0see @hoffie