readthedocs / sphinx_rtd_theme

Sphinx theme from Read the Docs
https://sphinx-rtd-theme.readthedocs.io/
MIT License
4.81k stars 1.74k forks source link

Rename master -> main, archive direct pip install #1221

Open agjohnson opened 3 years ago

agjohnson commented 3 years ago

We've been slowly moving to the new default branch name with our repositories and I just realized that this could be used as an opportunity to archive users ability to install directly from our repository. We'd have to play with how this works with pip, however what if we did this:

The big question that I'm not certain on is how do we make the default development branch main without also repointing the default branch pip installs from, master? This needs more thought.

hugovk commented 3 years ago

Most other repos rename master to main, so there's no longer an old master. GitHub will redirect from the old to the new.

This is neater, there's no old master branch to cause confusion.

GitHub has some tooling and instructions: https://github.com/github/renaming

The big question that I'm not certain on is how do we make the default development branch main without also repointing the default branch pip installs from, master? This needs more thought.

So you'd want to completely prevent people installing via Git? I'm not sure how to achieve that, other than perhaps keeping an old master branch, replace the contents with something un-installable?

How exactly do they install from Git now?

andersk commented 3 years ago

pip does not install from master, it installs from the default branch (what Git calls HEAD), which will be main if you rename it using GitHub’s instructions. For example, Flask’s default branch is main and this works:

pip install git+https://github.com/pallets/flask.git
agjohnson commented 3 years ago

So you'd want to completely prevent people installing via Git?

Yeah, we've discussed this as an option to remove the need to check in built assets to our repository. This flow currently makes review and merging difficult.

How exactly do they install from Git now?

Just via a standard:

pip install git+https://github.com/readthedocs/sphinx_rtd_theme.git

or similar. Most users do this to install a master branch.

pip does not install from master, it installs from the default branch (what Git calls HEAD), which will be main

Yeah, my question is more if GitHub has any options for us. I haven't researched much here. If we change the default branch on the repository, our development flow and review will all branch from main, but pip will also install from main. If there is an option to keep review branched from main but leave the default branch as master, that might solve the issue.

In all reality, this might be too complicated. It might be easier to move development to a separate repository, stop checking in built assets there, and continue building this repository (including checked in built assets) from a secondary repository for some period of time.

agjohnson commented 2 years ago

Also, worth mentioning here that on other repositories we are still checking in built assets to our repo during PRs. The pattern we have established here and in other repositories is to try rebuilding the assets in CI and raising a failure if the built assets are not up to date. This addresses contributors opening pull requests without assets rebuilt just fine it seems.

Conceptually, moving built assets outside of repositories is going to be fairly tricky:

So, it's worth weighing if removing built assets from our repository is a strong requirement. It originally was discussed as important because contributors kept opening pull requests without rebuilding assets. That is largely already solved and new contributions should get the idea after their pull request checks fail.

As a path forward, perhaps our opinion is that users can install from our default branch at their own risk, but should really be using a supported release, and we will continue checking in assets built with each pull request.

benjaoming commented 2 years ago

@agjohnson we decided to "freeze" the master branch and create a new default main branch, right?

I think the target milestone can be 2.0 for this change.

agjohnson commented 2 years ago

we decided to "freeze" the master branch and create a new default main branch, right?

I think you mentioned an idea you had here, but no, we did not decide to freeze/rename master yet.

You had described branching from master, but not updating the default branch on this repo. Is that the plan you're talking about? I'd say I'm -1 on that plan, it will lead to confusion with where to branch PRs from, PRs will look funny without always setting the base, etc.

To make master eventually go away, we don't need to freeze master, we will just rename the branch like we did on other repos.

I think we can hold off on this, it's not a sprint priority at the moment. We'll need to prioritize more work on the theme in some future sprints though.

To get back to this work, I'd first like to have an accurate idea of the number of installs on RTD that pointed directly to our repo (we have figures on this but the data looked wrong). We might be concerned about a marginal use case. I'd also like to experiment more with my points above, if necessary. There probably isn't anything there, and we'll just end up renaming master -> main though.

benjaoming commented 2 years ago

You had described branching from master, but not updating the default branch on this repo.

No, I'm also -1 on that :) The default branch should be main in some future, so I was wondering if the milestone could be set to 2.0 in order to move it a bit forwards and get rid of an installation method.

If we can simply get away with renaming the branch, I would go for that, too. But I'm not sure about the implications, especially I was wondering if this is actually an installation method we want to support in the future. For instance, we might want to remove build artifacts from the repo.

By "freezing" I mean to have a master branch but leave it at a certain stable version and never update it again. That way, we can put an "upper bound" on where we stopped supporting this installation method. If we rename the branch and use GitHub's redirect mechanism, we will have to continue supporting this installation method?

benjaoming commented 2 years ago

Sorry, you already stated it:

So, it's worth weighing if removing built assets from our repository is a strong requirement.

It seem that this decision needs to be made and put in the roadmap, then we can choose where to go with this one.

benjaoming commented 2 years ago

I would definitely be :+1: on removing the assets from the repo in 3.0 or 2.0, they are a big burden to maintain in PRs.

I think the benefit of putting the effort into building them as artifacts will be for maintainers and contributors. It can save a lot of time. I'm still picking up the routines, but I get a feeling that it's significant.