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

Only create one update PR for a dependency #367

Open
TSRBerry opened this issue May 19, 2023 · 10 comments
Open

Only create one update PR for a dependency #367

TSRBerry opened this issue May 19, 2023 · 10 comments

Comments

@TSRBerry
Copy link

Currently every day a new PR is opened on org.ryujinx.Ryujinx about the same update.

The flatpak-external-data-checker should check for an existing PR and update it if the dependency is the same, instead of creating a new one every time.

Having 7 open PRs for the same update is not optimal, especially since the person with write permissions to our repo is currently not available, so we will get even more open PRs for the same thing in the next few days.

I know ideally we should be merging them as soon as possible, but currently there is no way for us to do that.

@vchernin
Copy link
Contributor

Related #62 #178 #289

@wjt
Copy link
Contributor

wjt commented May 23, 2023

To give some detail:

The reason a new PR is opened multiple times every day is because another bot is updating the repo multiple times a day. PRs are deduplicated based on a hash of the tree content after applying the update #178 and because the repo keeps changing, so does the hash.

@TSRBerry
Copy link
Author

TSRBerry commented May 23, 2023

I see! Is there a reason it's not using a similar approach to dependabot where the branch name is basically botname/dependency-name/new-version-number instead of relying on git hashes?

@wjt
Copy link
Contributor

wjt commented May 24, 2023

One reason is that updates for multiple sources can be updated at once. But, yes, splitting updates by module and taking that approach would be a solution, at the cost of breaking the "important sources" feature https://github.com/flathub/flatpak-external-data-checker#selectively-submitting-prs

@Jehan
Copy link

Jehan commented Jun 7, 2023

The reason a new PR is opened multiple times every day is because another bot is updating the repo multiple times a day. PRs are deduplicated based on a hash of the tree content after applying the update #178 and because the repo keeps changing, so does the hash.

Our repo (org.gimp.GIMP) doesn't change (there is no bot updating it), yet we also have multiple duplicated pull request by the flathubbot. It's weird, sometimes it updates 4 deps, the next time 3, the next time 2 or even just 1 (if we didn't commit any change, I don't get why some dependencies disappear), then again 4 or 3 and so on. Anyway I just closed nearly 10 pull requests (only kept the latest). Same as @TSRBerry, I wished we were faster to review these patches, but it's not always possible, especially when you are a volunteer-based project.

So yeah, it would be nice eventually that the bot can detect previously opened requests and simply update it when needed.

P.S.: this dependency checker is very neat though, thanks for this. It could be even better by being this tiny bit "smarter", therefore less bothersome. 😉

@hfiguiere
Copy link

On this repo https://github.com/flathub/org.freedesktop.LinuxAudio.Plugins.Airwindows/pulls

it keep opening a PR, but unlike stated above, there is no change.

@wjt
Copy link
Contributor

wjt commented Jul 23, 2023

That's because the git checker can't determine the release date, so it always uses the current date, so every day the change is different.

There are examples in the Readme about using the GitHub API with the JSON checker rather than using the git checker.

(I believe there is no way with vanilla git to get the timestamp of a tag without cloning the whole repo. Personally I think it would be neat to recognise well-known git hosts, try to use their API, and fall back to cloning the repo, rather than guessing the date.)

@takluyver
Copy link

This just bit the us.zoom.Zoom flatpak. A new version had been found to crash on launch, so the PR was not merged, and we were discussing what the issue was and how to fix it. Then the bot opened an identical PR, one user reported it was working, and a busy maintainer merged it without realising it was the same as the update causing crashes.

@wjt
Copy link
Contributor

wjt commented Jan 12, 2024

Can you link to the two Zoom PRs?

@takluyver
Copy link

Sure: flathub/us.zoom.Zoom#429 & flathub/us.zoom.Zoom#436

I think it's getting a different hash because the base branch had changed in the meantime, as mentioned in #178.

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

6 participants