-
Notifications
You must be signed in to change notification settings - Fork 423
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
Add JuliaFormatter to workflow #1401
Comments
Seems not completely striaghtforward to update PRs (in particular for new contributors or people who are not very experienced with git), so I think we should not rush this. I think it would be nice to adopt a code style at some point but IMO it is not a top priority (many/most packages are fine without one) and the code looks quite reasonable already without enforcing a specific style. I think it would be better to take the time first to go through the existing PRs and drafts and finish or close them. |
In principle I'd agree, but I'm just wondering how likely is it that's actually going to happen ?
How many people allow "edits by maintainers"? It might be easier to just do it on behalf of the PR openers - then they;ll just have to git pull? |
I don't know, I check old issues and PRs from time to time at least. Even more so now that you bumped some of the PRs 😄 Also previously sometimes I finished PRs or opened alternatives such that the bug fixes or features become available.
I don't know either but apparently not everyone. I merged master into a PR locally today but was not allowed to push the changes. Seems I have to open a new PR. |
@devmotion @matbesancon how about joining Closember ? :) |
I don't understand the goal of it, I think number of closed issues or PRs is a terrible metric. I guess I might misunderstand something but it mainly reminds me of the Github bot that closes issues after some time automatically. IMHO this is a really bad idea. I also think one should not relate number of issues and PRs to well-being of maintainers. No maintainer is obliged to address issues or review or merge PRs, and it can't be demanded from them by any user. This blog post summarizes my view on issues and maintainers of opensource projects quite well: https://drewdevault.com/2021/10/26/stalebot.html That being said, it's always great if someone opens a PR that fixes an issue. I think, however, most open PRs were reviewed but are not merged yet because additional changes were requested. If a PR gets lost, it's good to bump it after while. |
Well, you were both talking about wanting to close PRs:
Which did strike me as leading to a game of whack-a-mole (given that new ones crop up all the time), and I just thought a concerted effort (for which something like Closember might provide some motivation) might help with that. Regarding your comment:
I agree that you're not obliged to add features (or even fix any issues). Anyone in the community can do that. But I do think by being a maintainer you take up the obligation to review and merge PRs - or to close them, if you think they aren't fit for reviewing&merging! The blog post you linked describes
Only maintainers can review or merge PRs. I appreciate this is something for which you're just volunteering your time! But if the nominal maintainers didn't review or merge PRs, then does that not leave a project effectively unmaintained? Leaving the community unable to actually contribute to it. Just speaking from my personal experience, I am keen to contribute bugfixes & features that improve compatibility between packages. But it's also me contributing my time, and it's pretty demotivating if the maintainers then don't actually respond to it. |
Originally posted by @matbesancon in #1283 (comment)
NB- the following workflow should only have a minimal amount of pain involved in each remaining open PR, assuming
format(".")
on master is committed as a single commit taggedformat_commit
:git checkout feature_branch
git merge format_commit^
(no conflicts, or actual content conflicts that must be resolved should be handled here)julia -e "using JuliaFormatter; format("."); git commit -am "PR format"
git merge format_commit; git checkout --ours .; git commit -a
(we know the only diff is the formatting, so it's safe to blanket-accept the local branch side)git merge master
(should apply cleanly now)Of course, PRs that would be straightforward to merge should probably be merged first. Here's a (not necessarily 100% correct nor complete) list of the ones that seem like they mostly need a "click merge":
minimum
,maximum
,extrema
forAbstractMvNormal
andProduct
#1319The text was updated successfully, but these errors were encountered: