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

Creation date of the draft Conventions document #457

Closed
larsbarring opened this issue Sep 18, 2023 · 10 comments · Fixed by #475
Closed

Creation date of the draft Conventions document #457

larsbarring opened this issue Sep 18, 2023 · 10 comments · Fixed by #475
Labels
defect Conventions text meaning not as intended, misleading, unclear, has typos, format or language errors

Comments

@larsbarring
Copy link
Contributor

Currently the creation date of the draft version of the Conventions document is the same as the recentmost released version e.g. today the draft version of CF-1.11 is "version 1.11 draft, 31 August, 2022", even though a PR was merged earlier today.

I imagine that it would be possible to somewhere in the document build process dynamically assign the correct date. But I afraid that the technical aspects for making this happen is out of my reach.

@larsbarring larsbarring added the defect Conventions text meaning not as intended, misleading, unclear, has typos, format or language errors label Sep 18, 2023
@JonathanGregory
Copy link
Contributor

I suppose that a simple fix for that is simply to delete the date in the working version. It can be replaced with some obvious placeholder, to be filled in when a new release is made. Would that be better?

@erget
Copy link
Member

erget commented Sep 19, 2023

Yes, that could work. Alternatively, we could have a date that is a placeholder in the AsciiDoc and gets turned into a date during build. I think this question is mostly for those responsible for the building, what they prefer.

@larsbarring
Copy link
Contributor Author

I have played around with this and will shortly create a pull request. In addition to minor changes to the AsciiDoc file, the github workflow for creating the documents have to be slightly modified (details in the PR).

@davidhassell
Copy link
Contributor

Hi Lars,

This looks good to me. I have tested it on my local build, and it looks fine for both HTML and PDF documents.

Might I suggest creating an environment variable for the resolved date that can be used in both HTML and PDF build commands? E.g.

    - name: Set draft date-time formatting
      if: github.event_name != 'release'
      run: |
        echo "docprodtime=-a docprodtime=$(date -u '+%d %B, %Y')" >> "$GITHUB_ENV"
    - name: Set "final" tag and date-time formatting
      if: github.event_name == 'release'
      run: |
        echo "FINAL_TAG=-a final" >> "$GITHUB_ENV"
    # Build cf-conventions.html using the Analog-inc asciidoctor-action
    - name: Build cf-conventions.html
      uses: Analog-inc/[email protected]
      with:
        shellcommand: 'asciidoctor --verbose ${FINAL_TAG} ${docprodtime} cf-conventions.adoc -D conventions_build; cp -r images conventions_build'
    # Build cf-conventions.pdf using the Analog-inc asciidoctor-action
    - name: Build cf-conventions.pdf
      uses: Analog-inc/[email protected]
      with:
        shellcommand: 'asciidoctor-pdf --verbose ${FINAL_TAG} -a docprodtime="$(date -u ${DATE_FMT})" -d book -a pdf-theme=default-theme-CF-version.yml --trace cf-conventions.adoc -D conventions_build'
    # Upload artifact containing cf-convent

@larsbarring
Copy link
Contributor Author

Yes, indeed, the environment variable should be used for both builds. And I an see that you are using a more advanced one than I was [able to figure out], which simplifies the asciidoctor arguments.

However, by not creating this variable in the if: github.event_name == 'release' branch it will only work for building the drafts and not when building the final versions (if I am not mistaken ?).

In my version I used slightly different time stamp formats for the draft and final versions. The former included hh:mm:ssZ because I thought that maybe there could possibly be more than one version in one day, but this might very well overdo it. In the final version the time was not included.

@larsbarring
Copy link
Contributor Author

I experimented a bit with creating the docprodtime environment variable as per your suggestion, but I couldn't get it working in the github workflow (nor locally). Seems that I couldn't get the single and double apostrophes right. As the current versions works I think that it is enough. What do you think @davidhassell ?

@larsbarring
Copy link
Contributor Author

What works well locally seems not to work on the runner. The following error appears when building both the html and pdf:
date: invalid date ''+%d %B, %Y %H:%M:%SZ''

@davidhassell
Copy link
Contributor

Hi Lars, Does the "invalid date" only appear when you introduce the environment variable? If so, then the other version that works is certainly fine by me.

@larsbarring
Copy link
Contributor Author

larsbarring commented Nov 22, 2023

It now works, see this screen clip:

image

The html and pdf documents produced when testing the PR are available for download as a zip file from the bottom of this page: https://github.com/cf-convention/cf-conventions/actions/runs/6955303266 (only useful for checking the outcome of the change).

The problem went away when I removed the apostrophes in the DATE_FMT assignments. With this I think the PR is ready to merge. @davidhassell would you mind having a final look and then merge if nothing more comes up?

@davidhassell
Copy link
Contributor

Hi Lars, Looks good - merging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
defect Conventions text meaning not as intended, misleading, unclear, has typos, format or language errors
Projects
None yet
4 participants