CellularResolutions.m2 pull request for JSAG (try 2)
This is the pull request for the CellularResolutions package for JSAG. The changes are only the author emails since the package was already previously included in the Macualay2 distribution.
See also, these previous pull requests: https://github.com/Macaulay2/M2/pull/2833, https://github.com/Macaulay2/M2/pull/2844
I've created a draft pull request this time so it won't get merged by accident. See #3968
CC: @OlaSobieska
Thanks, @jkyang92 (and @OlaSobieska) ! I'm pretty sure your package is in good shape, but let's see what comments we would collect in this PR --- this would be a good test case.
I will propose to have an M2Internals discussion of how to review a general JSAG-linked PR in various scenarios (new package, existing package, etc.) and possible outcomes (for the PR and for the JSAG submission).
but let's see what comments we would collect in this PR --- this would be a good test case.
As I mentioned here, GitHub doesn't allow comments on lines not changed in given PR, nor does it even show beyond a few lines surrounding the diff (e.g. I can see that this package has AuxiliaryFiles => true but only the main file is visible from this PR). Doesn't this make using GitHub PR review interface essentially pointless here?
An alternative is to also leave comments on the previously listed pull requests, but even then it's hard to leave a comment on v1 and check that is hasn't already been changed in v2, etc.
Doesn't this make using GitHub PR review interface essentially pointless here?
No, there are still plenty of options for constructive feedback.
No, there are still plenty of options for constructive feedback.
Do you mean just commenting as if this was an issue? Could you give an example of giving feedback using the PR review interface here?
No, there are still plenty of options for constructive feedback.
Do you mean just commenting as if this was an issue? Could you give an example of giving feedback using the PR review interface here?
Please (from now) comment only on the package associated to this PR here. I've created a topic on Zulip (M2Internals Discussions) to collect comments on the general JSAG M2 PR process.
Request: make your comments short and to the point and moderate/neutral in tone.
The code looks quite clean, but as we know every program can be made one line shorter. Perhaps this is the line: https://github.com/jkyang92/M2/blob/cellres-jsag/M2%2FMacaulay2%2Fpackages%2FCellularResolutions.m2#L357