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

chore: adding docs to release please #3571

Merged
merged 6 commits into from
Nov 30, 2023
Merged

Conversation

signorecello
Copy link
Contributor

Description

This PR attempts to add docs to release_please

Problem*

So far our attempts with docs workflows have been unsuccessful. The latest try was almost there, but it implied having a PR after the release is merged, which would actually make the docs show up after the tag, not before. So when the release was tagged as stable, it wouldn't have the docs at that point in time.

Summary*

This change attempts to do some blind changes, as I tried faking master with another branch and somehow didn't make it actually trigger new tags.

@github-actions github-actions bot added the documentation Improvements or additions to documentation label Nov 24, 2023
@TomAFrench TomAFrench changed the title feat: adding docs to release_please 🤞 feat: adding docs to release please Nov 24, 2023
@TomAFrench TomAFrench changed the title feat: adding docs to release please chore: adding docs to release please Nov 24, 2023
Copy link
Contributor

github-actions bot commented Nov 24, 2023

Copy link
Member

@TomAFrench TomAFrench left a comment

Choose a reason for hiding this comment

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

This will publish a new docs release every time the release please PR is updated rather than when it's merged. I can help make some changes to this on Monday.

@signorecello
Copy link
Contributor Author

Oh I see... Well I do appreciate the help, I'm struggling a bit with this.

The only way I can imagine is having a new version of the docs be built just before merging this PR, but still on this PR. I have no idea how to achieve that...

* master:
  feat: codegen typed interfaces for functions in `noir_codegen` (#3533)
  chore: add dependency on noir_js from docs package (#3559)
@TomAFrench
Copy link
Member

TomAFrench commented Nov 27, 2023

No worries, I've split the workflow into two steps:

  1. Whenever we update the release-please PR, we create a new version of the docs and commit it to the release please branch.
  2. When we merge the release please PR, that version then gets "frozen" and deployed to netlify.

That said, this PR locks us in to only allowing docs deployments in line with noir releases (i.e. to update the docs we need to push a new release of nargo). This doesn't seem quite right imo, shall we split off the publishing into a separate workflow again and allow an external trigger (or just redeploy on changes to the docs directory)?

@signorecello
Copy link
Contributor Author

signorecello commented Nov 27, 2023

Thank you so much @TomAFrench in regards to your question, one can always change the past versions in the versioned_docs folder (which is just a snapshot of the "docs" folder at a certain tag) so I think it's no big problem 👍 we do it sometimes when we find typos in earlier versions or something

As for "up-to-date" docs we can have a workflow that deploys them on every merge, since the "docs" folder is the "current/dev/latest" version, not the one the users see first (the one they see at first is the stable version)

Copy link
Collaborator

@Savio-Sou Savio-Sou left a comment

Choose a reason for hiding this comment

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

Acknowledging this is still a net positive versus where we are at.

We can address the bloat and foreseen pitfalls with subsequent Issues.

@TomAFrench
Copy link
Member

This is on my plate for today.

Copy link
Member

@TomAFrench TomAFrench 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 reasonable now. Whenever we merge a release-please PR now:

  1. We'll have a copy of the docs in versioned_docs which is correct as of the release commit.
  2. A new version of the docs will be published as of that commit
    • This means that the dev docs will be automatically up to date for the new prerelease. (Although do you not want this to be published on any changes to the docs?)

If we mark the release as stable down the line we can then push out a new version of the docs by running the workflow manually. We could attach this to the release event again however.

@kevaundray kevaundray added this pull request to the merge queue Nov 30, 2023
Merged via the queue into master with commit beaf402 Nov 30, 2023
33 checks passed
@kevaundray kevaundray deleted the zpedro/docs_release_please branch November 30, 2023 17:39
@Savio-Sou
Copy link
Collaborator

Thank you Tom!

We could attach this to the release event again however.

That wouldn't quite work, as (quoting @signorecello from Slack): "When a GitHub Actions workflow is triggered by a release event, it checks out the repository at the state it was in when the release was created."

However the manual trigger is great and likely good enough until we find a better solution.

@signorecello
Copy link
Contributor Author

I think the manual trigger would release it at the time of tagging, so it should work... But tbh my self-confidence in my CI skills is not great at this time 😂

@signorecello
Copy link
Contributor Author

Thank you so much Tom. This makes sense to me 😊

@TomAFrench
Copy link
Member

That wouldn't quite work, as (quoting @signorecello from Slack): "When a GitHub Actions workflow is triggered by a release event, it checks out the repository at the state it was in when the release was created."

By default this is true, but we can specify a different tag to checkout instead so that if the workflow is triggered by a release then it checks out the current master branch.

TomAFrench added a commit that referenced this pull request Dec 1, 2023
* master: (124 commits)
  chore: add env var for test updates in nargo_fmt (#3638)
  chore: remove unnecessary conversions from `add_constant` (#3651)
  feat: Complex slice inputs for dynamic slice builtins (#3617)
  feat: Add `FieldElement::from<usize>` implementation (#3647)
  chore: fix relative paths in reease please (#3646)
  chore: add integration test for verifying a recursive proof onchain (#3167)
  chore: adding docs to release please (#3571)
  chore(ci): Fix publishing of ACVM crates (#3645)
  chore(docs): address visibility issues in docs (#3643)
  chore: type formatting (#3618)
  fix: Restrict fill_internal_slices pass to acir functions (#3634)
  chore(docs): docs for v0.19.4 (#3601)
  feat: aztec-packages (#3626)
  chore: Move tests to the correct root (#3633)
  feat: Implement integer printing (#3577)
  fix: corrected the formatting of error message parameters in index out of bounds error (#3630)
  chore: Update ACIR artifacts (#3619)
  chore(debugger): Run debugger REPL in thread (#3611)
  chore: remove deprecated method (#3625)
  feat: Implement raw string literals (#3556)
  ...
TomAFrench added a commit that referenced this pull request Dec 1, 2023
* master: (28 commits)
  feat: reuse witnesses more when interacting with memory (#3658)
  chore(ci): stop committing new acir artifacts (#3654)
  chore: add env var for test updates in nargo_fmt (#3638)
  chore: remove unnecessary conversions from `add_constant` (#3651)
  feat: Complex slice inputs for dynamic slice builtins (#3617)
  feat: Add `FieldElement::from<usize>` implementation (#3647)
  chore: fix relative paths in reease please (#3646)
  chore: add integration test for verifying a recursive proof onchain (#3167)
  chore: adding docs to release please (#3571)
  chore(ci): Fix publishing of ACVM crates (#3645)
  chore(docs): address visibility issues in docs (#3643)
  chore: type formatting (#3618)
  fix: Restrict fill_internal_slices pass to acir functions (#3634)
  chore(docs): docs for v0.19.4 (#3601)
  feat: aztec-packages (#3626)
  chore: Move tests to the correct root (#3633)
  feat: Implement integer printing (#3577)
  fix: corrected the formatting of error message parameters in index out of bounds error (#3630)
  chore: Update ACIR artifacts (#3619)
  chore(debugger): Run debugger REPL in thread (#3611)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants