Improve readability by removing #defined strings#135
Open
paulbartell wants to merge 3 commits intoFreeRTOS:mainfrom
Open
Improve readability by removing #defined strings#135paulbartell wants to merge 3 commits intoFreeRTOS:mainfrom
paulbartell wants to merge 3 commits intoFreeRTOS:mainfrom
Conversation
paulbartell
force-pushed
the
remove-macro-defined-strings
branch
from
7407689 to
d834a62
Compare
kstribrnAmzn
reviewed
Aug 2, 2022
Member
kstribrnAmzn
left a comment
kstribrnAmzn
left a comment
There was a problem hiding this comment.
I'm just going to state my opinion here which I realize is different from many on our team for the record....
In general, I'm a fan of removing single character and one use macros. I'm not really a fan of removing multi-character, multi-use macros. I'm okay with removing length macros as long as there is context where the raw value is used.
Essentially I think the macro name gives context more than anything, and I don't want to lose this context as it may result in future new developers suffering a harsher learning curve.
/stepping-of-my-soap-box
paulbartell
force-pushed
the
remove-macro-defined-strings
branch
from
e82a892 to
b5f5963
Compare
paulbartell
force-pushed
the
remove-macro-defined-strings
branch
from
b5f5963 to
99b236b
Compare
archigup
approved these changes
Aug 3, 2022
kstribrnAmzn
approved these changes
Aug 3, 2022
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Remove macro-defined string constants that are used in a limited number of places or are obvious.