Skip to content

(Re-)move misplaced log output and counter initialisation #239

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jan 16, 2022
Merged

(Re-)move misplaced log output and counter initialisation #239

merged 1 commit into from
Jan 16, 2022

Conversation

@Olf0
Copy link
Contributor

@Olf0 Olf0 commented Jan 16, 2022

This enhances PR #238, while retaining its intention.

@Olf0 Olf0 requested a review from nephros January 16, 2022 14:38
@Olf0 Olf0 self-assigned this Jan 16, 2022
@Olf0
Copy link
Contributor Author

Olf0 commented Jan 16, 2022

Thank you for making me look at pm_apply, again.
I would prefer to output the final mangle-list, as in PR #239 (in contrast to your suggestion PR #238), because this is what ultimately is used.
But I fully agree with the fundamental observation which resulted in PR #238: Logging the mangle list as debug-output, even when it is not used, is confusing and superfluous. As for the DISABLE_MANGLING variable, which results in an empty mangle list (which is obvious). And the original output format, which makes empty variables not obvious.

Plus: Each time I look at this, I detect some other flaws:

  • Move the initialisation of the found variable where it belongs: Right before the loop.
  • Remove a superfluous space character.
  • Enhance a comment.

Are you O.K. with merging PR #239 to the Olf0-patch-3 branch and ultimately merge that into master?
And to close PR #238 unmerged.

This was referenced Jan 16, 2022
Closed
Merged
@Olf0 Olf0 merged commit f3df007 into Olf0-patch-3 Jan 16, 2022
@Olf0 Olf0 deleted the Olf0-patch-3.2b branch January 16, 2022 19:01
Olf0 added a commit that referenced this pull request Jan 16, 2022
@Olf0
pm_apply: Do convert multiple libpath references etc.
e80cfa5 
[pm_apply] Do convert multiple libpath references

[pm_apply]Simplify code

[pm_apply] Let log output be more expressive and consistent

[pm_apply] Enhance checks&balances and debug output

pm_apply: improve regexp (#237)

[pm_apply] (Re-)move misplaced log output and counter initialisation (#239)

[pm_apply] De-indent the whole function mangle_libpath
This was referenced Feb 15, 2022
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nephros nephros nephros approved these changes

Assignees

@Olf0 Olf0

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Olf0 @nephros