squid
squid copied to clipboard
squid-cache
Maintenance: source maintenance script improvements
- Allow passing astyle program name via ASTYLE environment variable.
- By default, use "astyle-3.1" if present or "astyle" otherwise.
- Add an option to format changed source files only.
- Detect more checksum programs.
No changes to the default behavior except for environments where a program named "astyle-3.1" is present but a program named "astyle" was previously used.
I have moved all documentation to the printUsage function.
I'm runnign some testing, there is a broken assumption in assembling debug-sections.txt
kinkie added S-waiting-for-reviewer and removed S-waiting-for-author labels 12 hours ago
Please do not forget to actually request the review via GitHub Review pane. The S-waiting-for-reviewer label cannot be used to search for PRs awaiting a specific person review, which is how some of us often search for what they should be reviewing... That label was created when GitHub did not support explicit review requests IIRC. Perhaps the S-waiting-for-reviewer label label should be completely removed now because GitHub handles the same state better?
@rousskov , we should be done with all pending requests for change. Can you please confirm?
@yadij I see all the points you raised are now marked as resolved. Was it so to your satisfaction?
yes, that's correct. That's also why I chose "none" in this context. But I have no special attachment to it
On Fri, May 20, 2022 at 8:44 AM Alex Rousskov @.***> wrote:
@.**** commented on this pull request.
In scripts/source-maintenance.sh https://github.com/squid-cache/squid/pull/830#discussion_r878290164:
if ! git diff --quiet; then echo "There are unstaged changes. This script may modify sources." echo "Stage changes to avoid permanent losses when things go bad." exit 1 fi
-# On squid-cache.org we have to use the python scripted md5sum -HOST=
hostname-if test "$HOST" = "squid-cache.org" ; then
- MD5="md5" +for Checksum in md5sum md5 shasum sha1sum none
In the configure.ac context you pasted, "none" is not a name of the program to run and fail, but a special value that means "not found". In AC_PATH_PROG terminology: not prog-to-check-for but value-if-not-found.
In this PR, you have worked on two loops for finding programs. One loop runs a special program that should fail. The other one does not -- it detects the case where all attempts failed without resorting to an always-failing trailing attempt. The latter approach is a lot cleaner (and can be argued to resemble the autoconf approach, just with an empty string instead of "none" as a special value).
— Reply to this email directly, view it on GitHub https://github.com/squid-cache/squid/pull/830#discussion_r878290164, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABHPVDACTIV3P3OPM6FCMELVK6XMJANCNFSM45OWDNVA . You are receiving this because you were mentioned.Message ID: @.***>
-- Francesco
Status update: This PR needs an update to resolve merge conflicts and @yadij review.
@kinkie, would you be able to work on that merge conflict resolution? IIRC, we need to remove MD5-related code from the script because the script no longer computes checksums (after 761dced). FWIW, I would keep the new generic function for binary searching even if it is only used once, but I certainly do not insist on us keeping it. Other adjustments may be needed as well.
Status update: This PR needs an update to resolve merge conflicts and @yadij review.
@kinkie, would you be able to work on that merge conflict resolution? IIRC, we need to remove MD5-related code from the script because the script no longer computes checksums (after 761dced). FWIW, I would keep the new generic function for binary searching even if it is only used once, but I certainly do not insist on us keeping it. Other adjustments may be needed as well.
Status update: No changes in status for more than a month. Still waiting for @yadij review and a merge conflict resolution.
@kinkie, do you want me to resolve that merge conflict?
I'll leave this PR open until PR1220 is approved to preserve edit history in case it's needed
Superseded by PR#1220 - it's the same code
It is not the same code (yet), but let's focus on #1220 from now on. You can just copy the (temporary) lost code from this PR to #1220.
I'll leave this PR open until PR1220 is approved to preserve edit history in case it's needed
Your call, but I do not think the open/closed state of a PR affects its edit history. It may affect status checks availability, but those checks are broken for this PR anyway.