Skip to content
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

Minting WPIDs #2

Open
AlexanderPico opened this issue Jul 22, 2023 · 9 comments
Open

Minting WPIDs #2

AlexanderPico opened this issue Jul 22, 2023 · 9 comments
Assignees

Comments

@AlexanderPico
Copy link
Member

AlexanderPico commented Jul 22, 2023

Proposal:

PRs containing new pathways (not edits of existing pathways) can be given a provisional ID "WP0001" to be used by parsing scripts to generate JSON, TSV and image files (e.g., WP0001-datanodes.tsv) as part of the PR review process.

When a new pathway PR is ready to be accepted and merged, that is when a separate GH Action will be triggered to perform the final steps, including minting a proper WPID. This can easily be done by the action simply by checking the latest WPID in wp-db at the time of merge and incrementing the number. The action would simply rename all the "WP0001" files at that time and insert the WPID into the .gpml as well. Then commit/push the files to wp-db and sync with the wp.github.io.

@AlexanderPico
Copy link
Member Author

AlexanderPico commented Nov 4, 2023

There can be multiple PRs open, each with files named "WP0001" and they won't conflict since each PR is handled independently during the initial automatic tests and review. They only require unique filenames immediately before merging.

And if a given author submits more than one new pathway, then we increment locally, w.r.t. their fork, "WP0002", etc. These are all recognizable as provisional and won't be conflicting within nor across author forks and PRs.

The merge actions will stack so that even if two or more merges are triggered immediately, the first will complete before the second one starts, thus the second one will see the latest WPID issued from the first one. The concurrency field supports sequential action processing.

@AlexanderPico AlexanderPico self-assigned this Nov 4, 2023
@AlexanderPico
Copy link
Member Author

Regarding PathVisio and using GH API, all commits should be made on WPID-specific branches so that independent PRs can be made by a given author.

For example, if I make a new pathway, it would be committed to my fork in the "WP0001" branch as "WP0001.gpml". A PR would be created for this new submission. While in review, I could edit an existing pathway "WP554". Changed would be committed a "WP554" branch and the PR would be separately handled.

@larsgw
Copy link

larsgw commented Nov 5, 2023

So the branches would need to match the WPID exactly? What about multiple edits to the same pathway, does the branch get rebased and re-used? And what about small edits to multiple pathways (e.g. updating the references of ~50 pathways)?

@AlexanderPico
Copy link
Member Author

AlexanderPico commented Nov 5, 2023

The proposal is a PR per author and pathway.

  • If same author makes 3 edits to same pathway, then those can all be reviewed together in a single PR.
  • If two authors make edits to the same pathway, then 2 separate PRs.
  • If one author makes edits two 3 different pathways, then 3 separate PRs.

The branches will be named by PathVisio at the time of submission, so we can devise any convention we want. For simplicity, I'm proposing to just name them the same as the WPID to help organize.

@mkutmon
Copy link
Member

mkutmon commented Nov 6, 2023

This should work.
The hardest cases are the ones where multiple authors edit the same pathway. Those will need to be resolved incrementally and might lead to conflicts that will need to be resolved manually, I assume.

@AlexanderPico
Copy link
Member Author

Cool. Fortunately, we have years of experience with the wiki and know that that rarely happens. Further, any changes to distinct parts of the GPML (especially GPML2022) will auto-merge via git.

Manual resolution should only be needed if two authors edit, say the description, within the same hour or so. And, if that (ever) happens, then it probably is worth manual attention :)

@AlexanderPico
Copy link
Member Author

AlexanderPico commented Nov 20, 2023

Extended Proposal:

For new pathways, after they arrive with a user-incremented, temporary WPID, e.g., WP0001 or WPX1 for the user's first submission via their fork (always with a preceding 0 or X). This ensures uniqueness within the constraints of what PathVisio "knows" at the time of submission.

Then, the PR processing workflow can assign a new temporary WPID, based on the PR number, e.g., WP010 or WPR10 for PR#10 (always with a preceding 0 or R). This ensures uniqueness across submissions and over time, which is particularly critical during the review period, especially if we're going to use Jekyll to serve the draft pathways on the website.

The final, official WPID would be assigned at time of PR acceptance.

Note: the preceding 0 (or character) is a great convention for identifying temporary WPIDs from official WPIDs.

@AlexanderPico
Copy link
Member Author

Ok. New idea...

What if we never formally "accept" pull requests? Then we don't have to worry about the WPID of the attached gpml file. We can instead use labels to denote the status of a PR, including an "accepted" label that would trigger an action to move all the generated files (including the GPML) to the right repos and with the right name. We can determine the correct WPID for new pathways at that time and not worry about the name prior to then.

@AlexanderPico
Copy link
Member Author

AlexanderPico commented Jan 5, 2024

  • New GPMLs from PV can initially be named pathwayTitle_username to ensure uniqueness during PR creation in user fork and wp-db repos, placed in a drafts/ folder.
    • on_pull_request.yml will assign temporary ID using the PR numbering: WP0_PR10 (Note: we need the “WP0” since lots of our scripts assume a WP\d+ pattern)
  • Edited GPMLs from PV can be named WPID_username to ensure uniqueness during PR, placed in pathways/WPID/ folder.
    • on_pull_request.yml will retain the WPID, but include the PR to maintain uniqueness: WP554_PR11

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

No branches or pull requests

3 participants