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

Fix README.txt for release packages #1916

Merged
merged 2 commits into from Sep 11, 2022
Merged

Fix README.txt for release packages #1916

merged 2 commits into from Sep 11, 2022

Conversation

Wengier
Copy link
Sponsor Collaborator

@Wengier Wengier commented Sep 10, 2022

I have noticed that all current packages including the release zip packages contain file README/README.TXT which claims they are development snapshot builds with some technical info. This PR adds a new README/README.TXT file as a brief introduction to the project for inclusion with the release packages (with tags like refs/tags/v0.79.0) instead of the file for development snapshot builds.

@Wengier Wengier self-assigned this Sep 10, 2022
@kcgen
Copy link
Member

kcgen commented Sep 10, 2022

Thanks @Wengier, the end result is good.

The README is already a template file consisting of common text plus placeholder variables populated from metadata at package-creation time:

docs/README.template
<snip>
Please report any bugs or issues to: https://github.com/%GITHUB_REPO%
...
branch:%GIT_BRANCH%
commit:%GIT_COMMIT%
...
Have fun!

(We've even parameterized the REPO, so this file is properly created in forks).

The PR's approach of adding a new release-only README and flip-flopping:

2022-09-10_07-39

doesn't make use of the template, which can be expanded with the improved wording in your new README, plus new field(s) to indicate if it's a release tag (or a development build).

Ideally we want the template fed from as much repo metadata as possible.

@kcgen
Copy link
Member

kcgen commented Sep 10, 2022

Regarding determining if the currently checkout is a release or not, I think that logic should be moved into the package creation script or a stand-alone script.

The PR does this using three duplicate and not-obvious shell lines in the CI code:

2022-09-10_07-37

The triple duplication needs to be collapsed and made more self-documenting, and be moved locally so it's as portable as the current package-creation script, which can be run locally (without needing CI).

@kcgen
Copy link
Member

kcgen commented Sep 10, 2022

Regarding determining if the commit is a release or not:

export opt=
if [[ "${{ github.ref }}" == "refs/tags/"* ]] && [[ "${{ github.ref }}" != *"-"* ]]; then export opt=-t; fi

The git describe --exact-match HEAD command will succeed and return the current commit's annotated tag value (such as v0.78.1, if we're on a release) or fail if we're on an untagged dev commit.

Something like this can describe the type of commit:

function describe_commit() {
  if version_tag=$(git describe --exact-match HEAD 2> /dev/null); then
    echo "release $version_tag"
  else
    echo "a development branch"
  fi
}

That could be put into a bash script or function and made a bit more self documenting:

commit_description=$(describe_commit)

This 'release vs. dev branch' aspect can be added to the README template, perhaps with something like:

This package contains%COMMIT_DESCRIPTION%

Which could be populated along with the other template population steps the package creation script currently performs, with something like:

sed -i -e "s|%COMMIT_DESCRIPTION%|$commit_description|"  "$readme_tmpl"

@kcgen
Copy link
Member

kcgen commented Sep 10, 2022

Regarding the PR's addition of a -t command-line flag to the creation script, this won't be needed once the package creation script can describe the current commit and populate the template with that result.

scripts/create-package.sh Outdated Show resolved Hide resolved
@kcgen kcgen added the documentation Improvements or additions to documentation label Sep 11, 2022
@kcgen kcgen added this to In progress in 0.79 release via automation Sep 11, 2022
Copy link
Member

@kcgen kcgen left a comment

Choose a reason for hiding this comment

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

Thanks @Wengier.
Just one comment regarding use of export.

scripts/create-package.sh Outdated Show resolved Hide resolved
Copy link
Member

@kcgen kcgen left a comment

Choose a reason for hiding this comment

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

Looks good!

@kcgen kcgen merged commit cf6c2ff into main Sep 11, 2022
0.79 release automation moved this from In progress to Done Sep 11, 2022
@Wengier Wengier deleted the ww/readme-fix branch September 15, 2022 05:38
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
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants