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

Readme: Fix build instructions #1075

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

JanMatCodasip
Copy link
Contributor

The Readme still contained old (pre-adoc) build instructions. Fix it.

Mention also the use of submodules which are needed for the documentation to build.

Copy link
Collaborator

@pdonahue-ventana pdonahue-ventana left a comment

Choose a reason for hiding this comment

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

This seems right at first glance. However, it's different than .github/workflows/verify.yml (which I'm not an expert on). Any thoughts on that? And while I'm looking in the .github directory, does .github/workflows/publish.yml need to be updated? It still has the texlive stuff in it.

@JanMatCodasip JanMatCodasip force-pushed the readme-build-instructions branch from 4951a5d to 9f5178c Compare October 21, 2024 05:07
The Readme contained the old (pre-adoc) build instructions.
Fix them to describe the current build process.

Mention also the use of submodules, which are needed
for the documentation to build.
@JanMatCodasip JanMatCodasip force-pushed the readme-build-instructions branch from 9f5178c to 5dc62af Compare October 21, 2024 05:15
@JanMatCodasip
Copy link
Contributor Author

JanMatCodasip commented Oct 21, 2024

Hello @pdonahue-ventana -

I have checked .github/workflows/verify.yml and it is correct and consistent with the updated Readme. Still, I have slightly expanded the build instructions in Readme to provide a little more guidance to the user.

I have also looked at .github/workflows/publish.yml and you are correct, this job is definitely outdated:

I propose to remove it (in another merge request: #1078).

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