-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Build: implement build output contract #9088
Comments
I think structure is mostly figured out here. We do need this build contract file eventually, as we'll also use this file to support upload of HTML tarballs. So, regardless of whether these file paths need to be customized by users during build time, we do need some dedicated file here eventually. A conversation for a second implementation, but I'd love to see some way to bring the contract file and I would advocate for a clearer and more unique file name than |
I wrote about why I think it's a good idea to have this in a separate file in #8190 (comment) |
That discussion seems limited to whether or not the configuration file is better in
We'd treat this as a build step, to surface the interaction between our build process and the contract file to users. Otherwise, it's a magical file that does something if we encounter it. |
Oh, I think I misunderstood your point originally then. I agree with |
It might be worth reviewing this plan and discussing more, but I think we lean towards starting with communicating/documenting the output paths and anything else strictly necessary for build.jobs/commands usage and maybe pausing on the rest of the contract file. @humitos is there anything else in the contract file besides the output paths the you feel is necessary? |
To clarify, it feels like we're defining the contract incrementally each time we implement things, instead of all at once. It feels like we're mostly defining things by convention, and using YAML config data for the rest currently. |
I'm not sure to understand what you are asking here. Can you expand? Currently, we only have implemented the "output path" (just The original idea of this was:
The only thing implemented currently is the |
What I'm ultimately asking is: do we even need a contract file? Output paths are required, but I don't see a need to make them configurable. Convention here seems like it's plenty, as long as we communicate these somewhere for users.
(We might need to specify a filename for other formats though, as we only support one file per output type, but that's another conversation) Past that, it's not really clear we have a need for a separate contract file or additional configuration at all perhaps. For example, you originally highlighted some ideas here: But these seem like they could either be in Are there any configuration options besides output paths that you feel are necessary @humitos ? |
Could it be determined whether a format will be built at runtime? For example, I could want to only build HTML for PRs to make the build take less time and allow to see the preview quicker but build all formats when building from the main branch or tag so that they're actually built when it matters. I imagine it could be possible simply by not creating the |
We need a contract file because we want to output a lot more information than just "where the output files are". If you are asking "do wee need a contract file for this particular case?". I'd say we may not --we are already doing this for
If we are using a convention for the PDF output's path, we can use a convention for this as well: "if you have multiple
I mean, yes. That's why I wrote the original design doc 1. I feel that we are recovering that old long conversation we had here. If that's the plan, I need to sit down again and re-read all the documents and notes we have around; but I'd ask other people to do the same so we are all in the same page because this "simple" question does not have a quick answer. I'm fine changing directions here if needed, since some things have changed already. However, I'd suggest to not do that without reviewing all those conversations and documents. Also, we are again breaking this conversation in multiple issues which makes everything more confusing. We are also talking about Footnotes
|
Well, eventually 😄 We have to make the rest of RTD aware of multiple PDF files first. I wouldn't take that work on yet, nor would I pre-optimize our implementation here for this constraint yet. It seems fine to start with support for single files first in our convention -- We can update our convention to support multiple files later, but it's still not clear we need secondary configuration here.
I'm mostly asking for clarification, not a change in direction. We started a change in direction already by moving towards convention over configuration. This provides users with the same functionality in a much easier to maintain way for us, and likely is easier for users to use as well. I think is an important point. I was asking about the other configuration options specifically above. I noted that it's not clear anymore why those need to be in a secondary configuration file. We already have trouble with one configuration file, two configuration files seems like it will slow us down too much. But with part of this contract already defined by convention, we'd need a strong reason to implement the remaining options in a secondary configuration file and take on this work in our roadmap.
I'm discussing this at a top level here as the rest of the contract file is something we need to place/remove from our roadmap. I think discussion around |
I'm fine using conventions where it's possible. However, if we want more people/doctools to integrate with us, we need to give them flexibility instead of forcing them to do things exactly as we want. There is a trade off here, of course. I don't have a strong preference for a separate file. I think it's enough with your idea of using a
An example of what I'm thinking here: build:
jobs:
# Override default jobs:
# It won't install "readthedocs-sphinx-ext" for example
install:
- pip install -r docs/requirements.txt
# It won't run the default "sphinx-build" command for HTML nor PDF,
# but a modified one by the user.
# It uses conventions for the output directory.
html:
- python -m sphinx -b html docs/ _readthedocs/html
pdf:
- sphinx-build -b simplepdf docs/ _readthedocs/pdf/
pre_metadata: null
metadata:
# Telemetry
doctool:
name: sphinx
version: 5.0.1
builder: html
extensions:
- autoapi
- hoverxref
theme: furo
# Hosting integrations
readthedocs:
# It uses a convention to inject it in the following div
# <div id="readthedocs-embed-flyout"></div>
flyout: true
# Here we need to figure it out how we are going to implement
# all the integrations we have in `readthedocs-sphinx-ext` since
# people is not going to install this extension anymore.
# These integrations have to be migrated into Javascript and enabled
# and configured from here.
post_metadata: null Note that I'm using The exact contract structure will be defined from the research we are doing in https://github.com/readthedocs/meta/issues/85#issuecomment-1329920589 and also in #9755 (comment). IMO, the first step here is migrating all the injections performed at build time to live in a Javascript library; being it the only file we inject on all the pages. |
Looking very far out, I agree that configuration could be a polished experience for users/extension developers. However, what we've implemented so far certainly works as well. The next places to invest our time for supporting third party tools will be in documentation UI/UX parity -- build jobs/commands parity, flyout injection, etc. Compared to a build contract concept, these pieces are much more limiting for users. If the contract isn't required for these pieces, then focusing on a contract file prematurely will delay working on these more important pieces. We could still take on the contract file later in the roadmap, but I'd probably want to see more signs of adoption of third party tooling before we worry about first class support for tooling developers. This was why I was advocating for experimenting with third party build tooling as much as possible, and regularly highlighting and promoting the results. Experimenting for our purposes is important, but we can't expect users to run with exploration here without some guidance. I doubt most users even know this is possible still. The work you were doing with https://github.com/readthedocs/meta/discussions/35 was great in this regard. I suppose I don't really share the vision in halting that work.
I am not necessarily arguing for either option here, I'm probably arguing for neither in fact. At least for the short term, and until we know more about what uptake will look like. Currently, we aren't seeing much uptake on third party tools, so we do have some growth here still. However, there are some strong points for implementing what we can from our build process as isolated, independent scripts/commands, and probably using build.jobs as much as possible. That is search indexing and flyout injection (or whatever we want) might all be candidates for implementing as an isolated script and separate build.job. This gives users potential control over these steps if they need. If we tuck this implementation back in the application, we usually get bogged down in premature polish discussion and fiddly implementations, and the implementation isn't extensible. This is just a half-baked idea on my part though, not a serious constraint, but perhaps warrants some thought.
Perhaps the current file is still a good place for this data, and this is a pattern to bring to other implementations. However, we are still a ways off from having community maintained support for third party tools, and we'd have to consider if we want community provided telemetry even. For now, we're still relying on users to maintain their own integrations, and we almost certainly can't rely on users to give us our own telemetry data. So I think customization of telemetry data collected might also be a good implementation to save for later? |
100% agree on this. Hosting integrations will give users a way better experience than a build contract just for supporting other tools1. Besides, I also think these integrations will guide the development of the build contract and make it a lot easier to design and implement. To clarify, the more configuration/conventions we can move to the Javascript library required (+ HTML structure) for the hosting integration, the better in my opinion. At this point, we need to define if there are configurations/values we have to inject at build time for the Javascript library to use when serving the page. That's probably "the smallest contract" we have to discuss now --and that's the work I was trying to do at #9755 which is kind of blocked on "defining how this Javascript library will work" Maybe it's time for another long meeting where you sit down in your porche, I prepare some mate and we talk for hours? 😄
Sure, I'm not saying we have to implement it now or anything similar, just to keep in mind when working on the design because this is something that we will eventually want to have. Footnotes
|
I'm with you here. I feel like we've seeded this conversation, but I feel like we need a map of all of these pieces at some point. I get a bit overwhelmed considering everything.
Hah, well my porch is covered in snow now, but my office is cozy! 😆 This would be a great place to go deep on pairing. I personally don't have this new build architecture model visualized yet and I think we'd all benefit from having this mapped out. Relating to our conversation this morning, we need to start defining what build pieces are moving, where they are moving to, and how we'll support configuration for them. We've thrown around some really good ideas here, but it seems we're all unsure of the next steps. Maybe to seed this conversation, I could share a few code examples of what front end configuration might look like. It would be similar to the EA client I think, in a lot of ways. This might help give some way to discuss technical patterns, and maybe we can work from what patterns we think users would like (or what configuration should live in our configuration, in build conventions, or in a metadata contract).
Cool, I think we're on the same page 🤝 And to clarify, I'm not at all opposed to a contract file. I do think it's a great plan, but I'm also excited to work on the implementation rearchitecture first, if possible. The contract file seems like it will be a natural progression, and we very well might just suddenly find it required, and that would still be a great outcome. |
Cool 💯 . I think this is a great next step. Knowing this will let us know (me, in particular) where those configuration has to come from (e.g. static in the YAML or dynamic at build time) |
An initial implementation putting all these pieces together is happening at #10127. I'm closing this issue to avoid confusions for now. We should continue the conversation there in the new PR and come back here if we need some extra old context. After re-reading this issue completely again, it seems the most important doubt yet is if there is required a new YAML file generated during the build process or not. In case it's required, there are some ideas here about how to get this data:
This "is this file required?" question was risen again in #10127. So, I think it's gonna be better to continue that discussion there instead. |
This is what so far has been called "metadata.yaml".
@humitos has the most background on what this work is.
Originally discussed in a draft PR here: #8190 and some notes on hackmd from a meeting.
The text was updated successfully, but these errors were encountered: