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

Option: all changes in single PR #126

Open
DilumAluthge opened this issue Nov 17, 2019 · 11 comments
Open

Option: all changes in single PR #126

DilumAluthge opened this issue Nov 17, 2019 · 11 comments

Comments

@DilumAluthge
Copy link
Member

No description provided.

@ericphanson
Copy link
Member

any chance the v3 rewrite can include this? 😄

@fchorney
Copy link
Collaborator

I would like to include this in the v3 update. One question, do we think this should always be on, or opt-in? This will somewhat determine how I make this work.

Personally I think it should just always be one PR, but I think this may potentially lead to issues. Lets say we need to update compats for A and B and make a single pr saying A and B need updating. If that PR doesn't get addressed, and then compats for C also needs an update, Compathelper will make a separate PR saying A, B, and C needs to be updated. Not a huge deal but sort of annoying? I suppose compathelper could potentially be smarter by checking the titles of the PRs to isolate a new PR for package C only?

Any other sort of edge cases you can think of?

@ericphanson
Copy link
Member

Sorry for the late reply. I think one PR sounds good, maybe could be opt-out if you want to give folks the old way too.

Lets say we need to update compats for A and B and make a single pr saying A and B need updating. If that PR doesn't get addressed, and then compats for C also needs an update, Compathelper will make a separate PR saying A, B, and C needs to be updated. Not a huge deal but sort of annoying? I suppose compathelper could potentially be smarter by checking the titles of the PRs to isolate a new PR for package C only?

It might be nice if it just updated the existing PR to add C to it too. So there's always just 1 PR from CompatHelper, instead of them piling up.

@DilumAluthge
Copy link
Member Author

At least for now, I want this to be opt-in.

By default, Dependabot creates individual PRs for each dependency, and CompatHelper very loosely tries to emulate Dependabot.

@DilumAluthge
Copy link
Member Author

For v3, let's keep it opt-in.

And then for v4, we can think about changing it to opt-out.

@fchorney
Copy link
Collaborator

I don't think i'll be able to get this done for the v3 merge in today, but would like to get it working in the next little while. Might be nice to keep thinking about the edge cases and exactly how it might work. I do like the idea of it updating a merge request to keep it up to date.

@simonbyrne
Copy link

It would be great if this could be the case for the first time it is run (which is typically when one sees a large number of PRs).

@MarkNahabedian
Copy link

I think it should be opt-out. That another tool got it wrong (in my opinion) is no reason to keep it wrong.

Regarding the A and B then C issue raised by @fchorney, a simpler approach than modifying a pending CompatHelper PR as suggested by @ericphanson is to exit early, doing nothing. This defers any new changes ComoatHelper would make until it's previous PR was merged.

I just don't want any additional goals to get in the way of the "single PR" improvement. The present behavior is unbearable when one has created a new project. I'm seriously considering not enabling CompatHelper in future projects until this is fixed.

Alternatively, perhaps there could be a tool one could run manually on one's local clone to set compat entries.

@DilumAluthge
Copy link
Member Author

Alternatively, perhaps there could be a tool one could run manually on one's local clone to set compat entries.

See e.g. https://github.com/GunnarFarneback/PackageCompatUI.jl

@DilumAluthge
Copy link
Member Author

Alternatively, perhaps there could be a tool one could run manually on one's local clone to set compat entries.

See e.g. https://github.com/GunnarFarneback/PackageCompatUI.jl

On Julia 1.8+, also see https://pkgdocs.julialang.org/dev/repl/#repl-compat

@MarkNahabedian
Copy link

MarkNahabedian commented Feb 6, 2023 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants