-
Notifications
You must be signed in to change notification settings - Fork 3.6k
subversion: polish Makefile #9667
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
Conversation
867f811
to
e1b6d0e
Compare
I am okay with adding PKG_CPE_ID and with replacing tabs with spaces in "define Package/subversion/Default/description". I see no added value in changing the order of variables or merging a double-liner into a runaway one-liner. IMO such changes only increase the size of the Git repo and pollute the source code history. Until OpenWrt publishes guidelines on the order of variables in Makefile, I am against making such changes that bring little or no value while increasing the size of the repo. |
@val-kulkov OK, I can remove those changes. It seemed logical to me to squeeze that changes into one PR, so stable and master package versions are more similar. When it comes to variable order, I'm trying to stick with suggested template from @BKPepe #9399 (comment) |
@ja-pa : I am not saying that I know better. IMHO the clean code history and the size of the repo are important. IMHO in this particular case, they outweigh the benefit of making the master Makefile and the LTS Makefile more similar. |
Why does the size matter? ./scripts/feeds/update makes a shallow clone. |
@neheb : indeed it does not matter if your goal is to compile a package. But if your goal is to review code history, it does matter. You cannot review code history from a shallow clone. Besides, IMO a non-functional re-arrangement of lines is a distraction that makes the code more time-consuming to review. |
OTOH it makes it more consistent with other packages. Makes for easier reading between them. |
e1b6d0e
to
006f1e5
Compare
@val-kulkov Ok, so I revert variable order changes and create an issue where we could continue with discussion #9682 |
One last thing that many of us tend to forget about: PKG_RELEASE. Please increment PKG_RELEASE and then everything should be good to go. Thank you. |
79f1dd0
to
9a092ef
Compare
@val-kulkov Updated. |
Changes: Description replace tabs Add PKG_CPE_ID Signed-off-by: Jan Pavlinec <jan.pavlinec@nic.cz>
LGTM. |
Maintainer: @val-kulkov
Compile tested: N/A
Run tested: N/A
Description:
This PR adds PKG_CPE_ID to Makefile variables since version 1.12.2 fixed two security issues
Release note and polishes Makefile similarly to #9666
Changes:
Polish variable order
Add PKG_CPE_ID
Signed-off-by: Jan Pavlinec jan.pavlinec@nic.cz