On Wed, 25 Aug 2021 at 09:22, Aaron Meurer <[email protected]> wrote:
> On Tue, Aug 24, 2021 at 6:30 PM Oscar Benjamin > <[email protected]> wrote: > > > > On Wed, 25 Aug 2021 at 01:07, Aaron Meurer <[email protected]> wrote: > > > > > > Are you simulating having the PR run in a "merged with master" state > > > when running the authors script on CI? Maybe we should update the > > > script itself so that it can do this. > > > > It's not a simulation. When a PR is pushed GitHub actions will make a > > temporary commit that merges the PR into master and then all tests run > > on that commit. The problem I'm referring to is that usually if you > > suggest that a contributor should "merge with master" in their PR the > > merge is in the opposite direction (merge master into the PR rather > > than PR into master). For most purposes this doesn't make any > > difference because the final state of the changes to files is the > > same. However the authors script looks through the topological order > > of the commits and that is not the same in both situations. That's why > > currently a rebase is needed: before the PR can be merged the > > topological order needs to correspond to the chronological order in > > which PRs are merged (but before the PRs are merged the *eventual* > > chronological order is not actually known). > > Right, the point is that the script currently doesn't give the same > results in a branch vs. that branch after being merged into master (or > rebased on top of master). But I think we could modify it so that it > does do this. I don't know if there's a fancy git command we can do, > but I think the simplest way would be to clone the repo into a temp > directory and merge the branch into master before running the git log > --topo-sort. > > But even so, I don't think this will solve the merge conflict problem > which, unless I am mistaken, will happen just from multiple people > adding names to the bottom of the file. > Yes, the merge conflict issue comes from insisting that many PRs need to make changes to the end of the same file. A simple fix is to make it so that the changes are not at the end e.g. by listing the names in alphabetical order. The bin/authors_update.py script has a load of code to carefully get the order right so presumably at some point someone thought that the ordering was important. It's possible that we could have the names temporarily stored in alphabetical order in a separate file that isn't included in the release. Then before release a script could collate the names and write them to the AUTHORS file in commit order. I wonder how other projects do this... Oscar -- You received this message because you are subscribed to the Google Groups "sympy" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/sympy/CAHVvXxR16TftRd2mRXrq6Nsdra5S%2BSYbvhtgGX%2Bf9Wmx4vAE9w%40mail.gmail.com.
