Skip to content
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

docs: update the readthedocs integration for PRs in this repo #65

Merged
merged 1 commit into from
Mar 6, 2025

Conversation

wmamills
Copy link
Collaborator

Upstream Read The Docs has tightened up the schema checking and this broke what we were doing for PRs.

A good number of things were tried but the best compromise is to:

  • supply a dummy sphinx config file in this repo
  • override the default build command to use the openamp-docs dir

Also:

  • add more documentation about what is going on
  • lock to html only format. PRs only do html anyway but make sure

@wmamills wmamills requested review from arnopo and tnmysh February 15, 2025 14:42
@wmamills wmamills self-assigned this Feb 15, 2025
@wmamills wmamills marked this pull request as draft February 15, 2025 14:42
@wmamills
Copy link
Collaborator Author

@arnopo @tnmysh I am assuming that the docs that are now in open-amp/doc/apps will be moved here to docs/apps.
If this is not true it is a bit silly to create the doc dir only to hold a do nothing file.
Let me know what you think

@arnopo
Copy link
Collaborator

arnopo commented Feb 17, 2025

@arnopo @tnmysh I am assuming that the docs that are now in open-amp/doc/apps will be moved here to docs/apps. If this is not true it is a bit silly to create the doc dir only to hold a do nothing file. Let me know what you think

Yes that's a good point this is part of remaining things to migrate.

@tnmysh
Copy link
Collaborator

tnmysh commented Feb 17, 2025

Should we open PR to track that migration?

@wmamills
Copy link
Collaborator Author

wmamills commented Feb 18, 2025

@tnmysh yes, please open an issue in open-amp
@arnopo do you both agree that this is the right thing to do? Move them as is from open-amp to openamp-system-reference?
The other alternative is to move the directly to openamp-docs.

@tnmysh
Copy link
Collaborator

tnmysh commented Feb 18, 2025

@tnmysh yes, please open an issue in open-amp @arnopo do you both agree that this is the right thing to do? Move them as is from open-amp to openamp-system-reference? The other alternative is to move the directly to openamp-docs.

I think keeping apps related docs in system-reference will be more helpful. Then openamp-docs can point link to this repo. But I am open to otherway as well.

@arnopo
Copy link
Collaborator

arnopo commented Feb 18, 2025

@tnmysh yes, please open an issue in open-amp @arnopo do you both agree that this is the right thing to do? Move them as is from open-amp to openamp-system-reference? The other alternative is to move the directly to openamp-docs.

I would be in favor of having complex documentation (for instance, PNG schematics) in the openamp-docs repo and just a README in the system-reference demo explaining how to build and run the demo.

And regarding @sipke's work on documentation, we probably have some duplications that should be cleaned up in the openamp-docs repo.

@tnmysh
Copy link
Collaborator

tnmysh commented Feb 19, 2025

open-amp issue is here: OpenAMP/open-amp#638

@wmamills
Copy link
Collaborator Author

wmamills commented Feb 20, 2025

Based on the discussion at OpenAMP system reference meeting on Feb 19th, we don't expect the doc/ directory in this repo to contain anything. I don't think we should create it just for this one file.

I suggest I rework this PR to move the doc/readthedocs_conf.py file to /.readthedocs_conf.py (in the root dir and with a leading dot)

@arnopo when you get back from vacation let me know if you agree

@wmamills wmamills marked this pull request as ready for review March 5, 2025 18:24
@wmamills
Copy link
Collaborator Author

wmamills commented Mar 5, 2025

@arnopo I have moved the config doc to the root dir (with a leading . ) and I don't create the doc dir.
I have tested this in my own account and it works OK.
I have removed the RFC status on this.

Upstream Read The Docs has tightened up the schema checking and this broke
what we were doing for PRs.

A good number of things were tried but the best compromise is to:
* supply a dummy sphinx config file in this repo
* as this repo does not have a doc/ dir we put the file in the root
* override the default build command to use the openamp-docs dir

Also:
* add more documentation about what is going on
* lock to html only format.  PRs only do html anyway but make sure

Signed-off-by: Bill Mills <[email protected]>
@wmamills
Copy link
Collaborator Author

wmamills commented Mar 5, 2025

I pushed again to add the leading . in the print statement:
print(".readthedocs-conf.py does nothing")

@arnopo arnopo merged commit 9df0e6b into OpenAMP:main Mar 6, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants