Skip to content

Restructire the POD of macros (parsers) #1267

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

Merged

Conversation

Alex-Jordan
Copy link
Contributor

This is part of an alternative to #1244, breaking it down into smaller PRs for review.

@@ -139,59 +138,54 @@ sub new {
return bless $f, $class;
}

##################################################
Copy link
Member

Choose a reason for hiding this comment

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

I often include separation lines like this to help group related methods together. I would prefer that they are left in, but, of course, the decision is up to you all. But I do find them helpful when I look at the code. Not everyone will.

@@ -1,3 +1,4 @@

Copy link
Member

Choose a reason for hiding this comment

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

Remove the empty line added here. I think this is the result of the comments that were an inappropriate discussion in the code, and that were later removed at my request. The comments were removed, but the empty line added below the comments were not.

Copy link
Member

@drgrice1 drgrice1 left a comment

Choose a reason for hiding this comment

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

I think that this pull request should perhaps be almost entirely reverted. That is everything except the actual POD changes. There is a lot of "clean up" of the comments in the code, and although I don't really like the comments like

#
# body of comment
#

i.e., the emtpy # lines before and after as those actually clutter the code and make the code less readable to me, these pull requests were intended to be POD clean up and restructuring, not comment clean up.

I can also see the usefulness of the full line ####### type comments that @dpvc commented on. Although, I still am not entirely favorable of those either.

In any case, that is not the point of the "POD restructuring".

@pstaabp
Copy link
Member

pstaabp commented Jul 19, 2025

I noticed that this was a bit of a mess when I went through it with the comment changes--I should have kept to the POD changes only.

I'll remove all of the comment changes from this and add a PR.

@pstaabp
Copy link
Member

pstaabp commented Jul 19, 2025

@Alex-Jordan Just added a PR to this.

@drgrice1 drgrice1 merged commit 56cfccf into openwebwork:PG-2.20 Jul 20, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants