jupyter / jupyter-sphinx

Sphinx extension for rendering of Jupyter interactive widgets.
https://jupyter-sphinx.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
187 stars 65 forks source link

Adapt to level config to match other Jupyter repos #237

Closed blink1073 closed 10 months ago

blink1073 commented 10 months ago
12rambau commented 10 months ago

I was looking at the repository history, and it seems normal "merge" has been used so far. For this PR only "squash and merge" is available, is it also something you changed @blink1073 ?

Note that if it's something we can change I am more in favor of a normal merge as it gives more credit to big contributions to the repository than small fixes and as long as the naming convention is correctly managed, it still allow the use of a meaningful changelog (the one generated from Github works just fine)

12rambau commented 10 months ago

@blink1073 I have reactivated the regular merge in the settings but I cannot set it back for this specific branch. Is it ok for you if I squash and merge so we can start from here for further development ?

blink1073 commented 10 months ago

I disabled "Allow merge commits" because they clutter up the history with merges from "main". Squash merging should be preferred IMO, unless there truly is some reason each commit in a PR has different meaning (which to me means that the PR should have been split). However, "Rebase merging" can be used in that scenario.

12rambau commented 10 months ago

@blink1073 I got an upvote from @akhmerov so I think this matter could be discussed. On my side I still see a huge disadvantage to squash merging: It gives the same recognition to a typo PR and a huge feature PR. As an example, I was dead inside when I realized that these 2 PRs (https://github.com/pydata/pydata-sphinx-theme/pull/540, https://github.com/pydata/pydata-sphinx-theme/pull/962/files) were given the same importance in my contributing section.

The problem you mention seems to be an egg and chicken issue as the "merge from main" conflict only occurs when you squash the previous PR. If not, git is very good at reconciling everything and Github provides a very convenient "sync" button that does everything for you from any branch.

Another example, I tried to contribute to a repository where people were less available for reviews (https://github.com/widgetti/ipyvuetify/pulls) but still using the same convention as the one you suggested: small single-feature PR. The consequence is that I have PR sitting there for 6 months preventing me from moving forward as I need them pushed sequentially. As this repository doesn't get as much maintainer love as the other Jupyter repos, I think it's very relevant to allow some motivated contributors to make big PR without squashing them.

And final arguments: it was the way things were done before here and everything seems ok so why changing it 😄 ?

blink1073 commented 10 months ago

I'm afk this week, and will defer to whatever you both think is best for this repo. My main point is that you can still see all the commits with the rebase option, but they are at least grouped. As long as the no-op "merge" commits aren't in the history I think it is fine.

12rambau commented 10 months ago

No worries, I'll pile some small PR in the meantime and wait for your return for the bigger ones. I tried to do it myself but I don't have access to branch policies and it seems the blocking parameter is coming from there.