search

goatcorp / Plogon

Build system for Dalamud Plugins
7 stars 13 forks source link

Use GitHub and GitLab diff url #26

Closed wolfcomp closed 1 year ago

wolfcomp commented 2 years ago

This fixes #23 for GitHub and GitLab

wolfcomp commented 1 year ago

Any feedback on this?

philpax commented 1 year ago

This looks good to me so I'm going to merge it.

philpax commented 1 year ago

Alright, we've had our first oddity:

https://github.com/goatcorp/DalamudPluginsD17/pull/402#issuecomment-1250377354

results in haveCommit becoming the empty tree commit (as it's empty), which doesn't work with GitHub. I think that's a separate, unrelated issue (it would be nice to get a diff between commits), but the main thing is that it just doesn't work.

I'd suggest opening up another PR to switch to linking to the repo instead of the diff if haveCommit doesn't exist?

wolfcomp commented 1 year ago

Another method could be to have the diff link to master%40%7B1day%7D instead of 4b825dc642cb6eb9a060e54bf8d69288fbee4904 for GitHub. Would need to check how this would work with GitLab to fix that one as I'm not sure entirely how that compare system works, as I just found the URL for where it is on the site digging through other repos.

wolfcomp commented 1 year ago

Actually found one better that works for both.

Initialize haveCommit with the return of git rev-list --max-parents=0 HEAD as this will return the first commit in the tree instead of using emptyTree

goaaats commented 1 year ago

Make sure that the diff actually has the files added in the first commit if you're going to do that, and test it please

philpax commented 1 year ago

My preference is still to link to the repo proper for an initial submission (which should be the only time the initial commit doesn't exist.)

Generating diffs is pointless / strictly worse than just looking at the repo with a web UI.