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

Add andino website #1

Merged
merged 23 commits into from
Jul 8, 2024
Merged

Add andino website #1

merged 23 commits into from
Jul 8, 2024

Conversation

JesusSilvaUtrera
Copy link
Collaborator

This PR is to add the general structure for the Andino website, not all the content of the pages is added. I will open it as a draft PR first so I can receive some feedback first. Some considerations:

  • I added some TODO for the future work.
  • The part of the Github Action is added but I can't test it and I don't know if it is well implemented (I am a total newbie with CI/CD), so any feedback there is appreciated.
  • ReadTheDocs build is not working right now, I have to take a look why, it said something about permissions but I am admin already in this repo.

CC: @francocipollone @jballoffet

@JesusSilvaUtrera
Copy link
Collaborator Author

Update: I am having trouble uploading the repo to ReadTheDocs. From waht I have seen, once you have connected your Github account, you should be able to import your repositories without any problem. The repo is found, but it takes the 'main' branch as default, so I wanted to change that in the Settings to take this add_andino_website instead as default branch to test if it worked, but as you can see in the image, it is not letting me change it:

image

@francocipollone do you know if this is expected or am I doing something wrong? We can just merge this PR and test it on the main branch directly, but I suppose there is a better solution.

@francocipollone
Copy link
Collaborator

@francocipollone do you know if this is expected or am I doing something wrong? We can just merge this PR and test it on the main branch directly, but I suppose there is a better solution.

Mjm, it might related to the fact that it doesn't recognize the content of the branch as something that can be uploadable.
If you want just to try that, create a bare minimum content to a separate branch (with some index.html page) and see if it appears in the list of options

btw: Feel free to invite me as admin of the project in readthedocs so maybe I can help you out

Copy link
Collaborator

@francocipollone francocipollone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that when you use readthedocs the sphinx build process is executed by readthedocs itself, so it is a bit different than when you use gh pages for instance.

See here https://github.com/maliput/maliput_documentation a good example on how to prepare the branch to be obtained from the readthedocs (In this case though there is a colcon build first (for bringing some doxygen information) which we do NOT need so skip that part)

I am ok about setting up a meeting to help you a bit to understand the general architecture (gh workflows, sphinx, readthedocs) in case you need it, just ping me.

@JesusSilvaUtrera
Copy link
Collaborator Author

Mjm, it might related to the fact that it doesn't recognize the content of the branch as something that can be uploadable. If you want just to try that, create a bare minimum content to a separate branch (with some index.html page) and see if it appears in the list of options

btw: Feel free to invite me as admin of the project in readthedocs so maybe I can help you out

I have just invited you to the project. About having another branch with minimum content, it should be the same, because (at least from what I understood) readthedocs looks for the .readthedocs.yaml file to configure everything. I will take a look again to the maliput_documentation repo and see what I am missing.

@JesusSilvaUtrera JesusSilvaUtrera marked this pull request as ready for review May 29, 2024 13:03
@JesusSilvaUtrera JesusSilvaUtrera self-assigned this Jun 5, 2024
@JesusSilvaUtrera
Copy link
Collaborator Author

@francocipollone all the changes we commented have been addressed, so you can proceed to the revision and tell me if you have thought of anything else. I haven't migrated andino_hardware becuase it will be changed when the new assmbly process is finished.

Thanks in advance!

Copy link
Collaborator

@francocipollone francocipollone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great start for the docs! Thanks for pushing this!
I left only one nit to go

README.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@francocipollone francocipollone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

One extra thing, can we change the website URL to be andino.readthedocs?

@JesusSilvaUtrera
Copy link
Collaborator Author

JesusSilvaUtrera commented Jun 10, 2024

One extra thing, can we change the website URL to be andino.readthedocs?

@francocipollone Wouldn't we need to create that domain, set the SSL certificate and all of that to achieve this? I tried to add the domain in the Project but it is pending to add the SSL certificate.

I also found this as a workaround, but it is not recommended: https://docs.readthedocs.io/en/stable/faq.html#how-do-i-change-my-project-slug-the-url-your-docs-are-served-at

@francocipollone
Copy link
Collaborator

One extra thing, can we change the website URL to be andino.readthedocs?

@francocipollone Wouldn't we need to create that domain, set the SSL certificate and all of that to achieve this? I tried to add the domain in the Project but it is pending to add the SSL certificate.

I also found this as a workaround, but it is not recommended: https://docs.readthedocs.io/en/stable/faq.html#how-do-i-change-my-project-slug-the-url-your-docs-are-served-at

No need to setup an own domain.

In general it uses the project name + readthedocs.org.
THe issue is that we used andino_documentation as project name. I renamed this but I think that the URL hasn't changed.
I am ok with deleting the project in readthedocs and create it again using andino as name of the project so we get the andino.readthedocs domain

@JesusSilvaUtrera
Copy link
Collaborator Author

Aaah okay okay I understood you wrong, I will do that tomorrow when I start the day and let you know how it goes.

@JesusSilvaUtrera
Copy link
Collaborator Author

@francocipollone I have created the new project and invited you, I deleted the old webhook and added the new one, and the automatic building seems to be working fine:

image

However, the videos are not working anymore on the website, but they are still working locally, and I don't get why is this happening, any ideas on that?

@francocipollone
Copy link
Collaborator

francocipollone commented Jun 11, 2024

@francocipollone I have created the new project and invited you, I deleted the old webhook and added the new one, and the automatic building seems to be working fine:

However, the videos are not working anymore on the website, but they are still working locally, and I don't get why is this happening, any ideas on that?

I don't think it is related to the new project being added to the readthedocs because the old page is also broken with respect to the video:
https://andino-documentation.readthedocs.io/en/latest/

@JesusSilvaUtrera
Copy link
Collaborator Author

I don't think it is related to the new project being added to the readthedocs because the old page is also broken with respect to the video: https://andino-documentation.readthedocs.io/en/latest/

Mmm true, I would swear that they were working last time I checked them, I don't know what happened, all the photos seem fine, maybe it's just an issue with rtd and using videos, I will check it.

@JesusSilvaUtrera
Copy link
Collaborator Author

@francocipollone I have managed to fix the videos. Apparently, you can't make the videos work both locally and on RTD, because you need to specify the path differently in each case, I added that in the general README for the future, but now it should be ready to merge, PTAL, thanks!

Copy link
Collaborator

@francocipollone francocipollone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's merge it and continue working on top.
The basic setup is achieved which is to any new commit we will have an update in readthedocs.

The content is good so far, probably we will go back to the architecture.
So far so good! Thanks for pushing this!

LGTM

@JesusSilvaUtrera JesusSilvaUtrera merged commit 221377d into main Jul 8, 2024
@JesusSilvaUtrera JesusSilvaUtrera deleted the add_andino_website branch July 8, 2024 06:03
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.

2 participants