Skip to content

Migrate user-facing feature flags into the product #9290

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

Closed
humitos opened this issue Jun 2, 2022 · 8 comments · Fixed by #9377 or #10471
Closed

Migrate user-facing feature flags into the product #9290

humitos opened this issue Jun 2, 2022 · 8 comments · Fixed by #9377 or #10471
Assignees
Labels
Accepted Accepted issue on our roadmap Needed: documentation Documentation is required

Comments

@humitos
Copy link
Member

humitos commented Jun 2, 2022

I'm creating an issue from an old Trello card that we have, so we can prioritize this work easily.

Taking a look at https://docs.readthedocs.io/en/stable/feature-flags.html#feature-flags, there are still some feature flags that we can migrate/remove:

  • PIP_ALWAYS_UPGRADE: this can be replaced by build.jobs.post_create_environment: pip install -U pip
  • UPDATE_CONDA_STARTUP: replaced by build.jobs.pre_create_environment: conda install conda
  • CONDA_APPEND_CORE_REQUIREMENTS: I'm not sure there is an easy way to replace this, but we could suggest a bashy solution maybe?
  • DONT_OVERWRITE_SPHINX_CONTEXT: this one does not seem to be replaceable either
  • LIST_PACKAGES_INSTALLED_ENV: this can be done now via build.jobs.pre_build: pip freeze

I think we could add these examples to FAQs (or even the "Build customization" page) and remove them from the page linked.

@humitos humitos added Needed: documentation Documentation is required Accepted Accepted issue on our roadmap labels Jun 2, 2022
@humitos humitos moved this to Planned in 📍Roadmap Jun 2, 2022
@humitos
Copy link
Member Author

humitos commented Jun 8, 2022

UPDATE_CONDA_STARTUP: replaced by build.jobs.pre_create_environment: conda install conda

Note that this is causing issues like #9306

LIST_PACKAGES_INSTALLED_ENV: this can be done now via build.jobs.pre_build: pip freeze

For internal usage, we are now saving this into BuildData objects.

humitos added a commit that referenced this issue Jun 27, 2022
I'm removing these because we don't want users ask for them to use in their
projects. All of the ones removed can be achieved by using `build.jobs.*` config.

Closes #9290
Repository owner moved this from Planned to Done in 📍Roadmap Jun 30, 2022
@humitos
Copy link
Member Author

humitos commented Jun 30, 2022

I'm re-opening this issue because we want to migrate users off from them. This could include sending them an email with the instructions and date or opening PRs on their repositories.

@humitos humitos reopened this Jun 30, 2022
Repository owner moved this from Done to In progress in 📍Roadmap Jun 30, 2022
@ericholscher
Copy link
Member

Yep, we have some good conversation about this here: #9377

@humitos
Copy link
Member Author

humitos commented Jun 22, 2023

We've done a lot of work on #9779 to remove old/deprecate feature flags. We still have some of them in our code base, but they are there for different reasons.

Exposed to the users

  • CONDA_APPEND_CORE_REQUIREMENTS: this one still seems to be useful. We will be able to remove it once we don't install dependencies on behalf of the user (mock, pillow, recommonmark, mkdocs, readthedocs-sphinx-ext, sphinx, sphinx_rtd_theme). I'm fine keeping it for now until we stop installing these. This feature flag cloud be migrated to Projects: allow feature flag configuration in our config file #9698 if we want to do something about it.
  • DONT_CREATE_INDEX: this is exposed because some people may be using a static index.html page and they may require this feature flag (we have only 1 project using this flag). My position here is to remove this feature flag completely and also remove the logic to create an index.html page automatically. The instructions exposed on that page are not great (see
    if not self.project.has_feature(Feature.DONT_CREATE_INDEX):
    index_text = """
    Welcome to Read the Docs
    ------------------------
    This is an autogenerated index file.
    Please create an ``index.{ext}`` or ``README.{ext}`` file with your own content
    under the root (or ``/docs``) directory in your repository.
    If you want to use another markup, choose a different builder in your settings.
    Check out our `Getting Started Guide
    <https://docs.readthedocs.io/en/latest/getting_started.html>`_ to become more
    familiar with Read the Docs.
    """
    ) and I think we can solve it better in a different way (e.g. in the 404 handler we can detect if it's the index.html from the root URL and expose more guidance there --point to the tutorial maybe?, or simple just don't show anything). This is aligned with our philosophy that avoids doing things on behalf of the users in the build process.

Require migration path

There is a nice table at #9779 (comment) that we can use to continue the conversation of these feature flag that require some work together with the users (e.g. onsite/email notifications, open PRs on their repositories, etc)

@ericholscher
Copy link
Member

ericholscher commented Jun 22, 2023

Yea, I'd like to get rid of the index.html generation. It's definitely from an older time of the platform, and the more we can simplify our builds the better. We should just raise a warning to the user in a notification if they don't generate an index.html on build... or just fail the build?

@humitos
Copy link
Member Author

humitos commented Jun 26, 2023

Yeah. Failing the build is the easiest and also it's going to be pretty clear to users what the problem is. I doubt there are valid projects serving proper documentation without an index.html file. So, I'm fine failing the build. I'll open a PR about this.

@humitos humitos moved this from In progress to Needs review in 📍Roadmap Jun 26, 2023
@github-project-automation github-project-automation bot moved this from Needs review to Done in 📍Roadmap Jun 28, 2023
@ericholscher
Copy link
Member

Is this fully done, or did it get auto-closed? 🤔

@humitos
Copy link
Member Author

humitos commented Jun 29, 2023

I think this is done already. We are using #9779 to track the deprecation emails and more. I think it's fine to close this issue at this point.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Accepted Accepted issue on our roadmap Needed: documentation Documentation is required
Projects
Archived in project
3 participants