Update GitHub Action for MarkBind

[tlylt]

What is the purpose of this pull request?

• Documentation update
• Bug fix
• Feature addition or enhancement
• Code maintenance
• Others, please explain:
  DevOps

Overview of changes:
Fixes #1461
Fixes #1004
(Supercedes and therefore) closes #1426

Although there is an existing GitHub Action markbind-action, it has certain limitations (e.g cannot specify the version of MarkBind to build files or prevent force-pushing when updating the gh-pages branch). Thus, in view of the work that I am doing to facilitate creation of GitHub workflows for PR-Preview, I am making this new GitHub Action markbind-deploy to be the "upgraded" version of 'markbind-action'.

The details of what it does can be found in the readme. In gist, it is a composite action that

• allows deployment with gh-pages and Surge
• can specify the version of MarkBind, including 'master' to choose the latest development MarkBind
• can be used for pr preview when following a branching workflow (for forking workflow, I am proposing two reusable workflows to tackle that, in view of security concerns. It's currently work-in-progress)
• can pass in markbind build arguments to handle cases of markbind site not at the root level (e.g. in /docs)
• will not force-push

Anything you'd like to highlight / discuss:

• The new GitHub Action should probably reside in MarkBind, whether to override 'markbind-action' or create a new repo 'markbind-deploy', up for discussion.
• Feedback on the approach, the configuration provided, the docs... is welcomed.
• Although the versioned docs feature is not ready, I feel that this is a standalone Action that does not need to wait for the versioning to work.
• The docs changes in this PR are not final, because the current workflow example is still using 'tlylt/markbind-deploy@main', which needs to be updated later on.
• There is one issue with site.json reported in Unable to serve/build with a site config file from another directory #1825, also affects this action
• This action is not published yet, but can be used for testing with 'tlylt/markbind-deploy@main'

Testing instructions:
I have tested a few scenarios, but please feel free to help test as well:
Referencing some of the Action runs I did to test this feature:

• PR preview
  • workflow file
  • action run
• Deploy to GitHub Pages
  • with diff version
  • with master branch MarkBind
  • with subfolder
• Deploy to Surge
  • workflow file
  • action run

Proposed commit message: (wrap lines at 72 characters)
Update documentation on MarkBind GitHub Actions

The existing GitHub Action "markbind-action" has been
revamped to improve its functionalities.

Let's add a reference to "markbind-action" in our
user guide to highlight this helper workflow. Let's also
include a section in our developer guide to explain how
to develop and maintain the "markbind-action" repo.

This helps users discover "markbind-action" and use it in
their CI/CD workflow. The dev docs will help developers
continue to improve "markbind-action"

---

Checklist: ☑️

• Updated the documentation for feature additions and enhancements
• Added tests for bug fixes or features
• Linked all related issues
• No unrelated changes

[jonahtanjz]

Nice work overall @tlylt!

Just some initial thoughts and comments.

The new GitHub Action should probably reside in MarkBind, whether to override 'markbind-action' or create a new repo 'markbind-deploy', up for discussion.

I feel that since this new workflow can do the same thing as 'markbind-action', it is fine to override it and publish a new version over it. Current users referencing this action in their workflow should still be able to use it if we keep the v1.0.0 release. This will also help to keep all the actions consolidated in one repository so it's easier to manage and keep track.

will not force-push

Not sure how useful it will be but perhaps we can also consider including a force-push option just in case some users would want to do that?

Feedback on the approach, the configuration provided, the docs... is welcomed.
I have tested a few scenarios, but please feel free to help test as well:

Overall it looks good, haven't tested it out yet. Will add more comments and feedback later!

@ryoarmanda and @raysonkoh, feel free to add in your thoughts on this as well :)

[tlylt]

will not force-push

Not sure how useful it will be but perhaps we can also consider including a force-push option just in case some users would want to do that?

I think the problem of "force-push" was because the original action will only keep the latest history like this, with only one commit.

The underlying Github action that I am currently utilizing for GitHub pages deployment will keep the commits and hence it is easy to refer to the previous versions of the branch. See here for an example. I think this makes the "force-push" issue irrelevant.

Nonetheless, I have added a keepFiles option to make use of the same feature in the underlying Github action, just in case this is useful.

[jonahtanjz]

Tested out a few scenarios and it seems to be working well in general.

Just 2 comments/suggestions:

1. For the baseUrl, it seems that if I leave this blank, it will assume that the baseUrl is "", which will overwrite the one specified in site.json. Would be nice if baseUrl will default to the one in site.json instead (in other words, use the markbind build command without the --baseUrl option if baseUrl is not set).

2. The version option doesn't seem to be working well. For example if I use v3.0.0, I can see in the CI log that v3.0.0 was used to build to site.

However, on the deployed site, I can see that the latest version, v3.1.1 (from the footer), was used instead to deploy the site.

Also tested the centre align syntax which was only released in v3.1.0 but it seems to work as well which means that v3.1.1 was actually used instead.

index.md with centre align code

[tlylt]

@jonahtanjz Thank you for the look-through!

1. For the baseUrl, it seems that if I leave this blank, it will assume that the baseUrl is "", which will overwrite the one specified in site.json. Would be nice if baseUrl will default to the one in site.json instead (in other words, use the markbind build command without the --baseUrl option if baseUrl is not set).

Sure let me see if this can be done.

2. The version option doesn't seem to be working well. For example if I use v3.0.0, I can see in the CI log that v3.0.0 was used to build to site.

I had the same issue initially but when I tested with v2 something, it was working fine. So I thought it must be some glitch.

Turns out this is a "bug" due to the version specification of our markbind-cli package. In the cli/package.json of the [email protected]:

"@markbind/core": "^3.1.0",
"@markbind/core-web": "^3.1.0",

When installing, this retrieves markbind/core of v3.1.1, because of the ^.
Thus when identifying the version number, the markbind build takes the version of the package.json of @markbind, which is v3.1.1

[jonahtanjz]

Turns out this is a "bug" due to the version specification of our markbind-cli package.

Thanks for investigating this @tlylt 👍

Yup seems like this is the issue as running npm i -g [email protected] will also install v3.1.1 of core and core-web locally. We can probably remove the ^ when referencing MarkBind's own packages for consistency. When a user installs v3.0.0 for the CLI, the rest of the packages should be v3.0.0 as well. If not, this will be a little confusing as the CLI will state "v3.0.0" but a different version is shown in the built site.

Can probably open a new issue and shift the discussion there :)

[tlylt]

1. For the baseUrl, it seems that if I leave this blank, it will assume that the baseUrl is "", which will overwrite the one specified in site.json. Would be nice if baseUrl will default to the one in site.json instead (in other words, use the markbind build command without the --baseUrl option if baseUrl is not set).

Updated this such that if no 'baseUrl' is set, it will default to the attribute in site.json.

[ang-zeyu]

The new GitHub Action should probably reside in MarkBind, whether to override 'markbind-action' or create a new repo 'markbind-deploy', up for discussion.

I feel that since this new workflow can do the same thing as 'markbind-action', it is fine to override it and publish a new version over it. Current users referencing this action in their workflow should still be able to use it if we keep the v1.0.0 release. This will also help to keep all the actions consolidated in one repository so it's easier to manage and keep track.

Think it would be good to reduce repo clutter as well, let's put the PR up over at https://github.com/MarkBind/markbind-action @tlylt. We can publish this as a major version since it has a few significant changes.

[tlylt]

Think it would be good to reduce repo clutter as well, let's put the PR up over at MarkBind/markbind-action @tlylt. We can publish this as a major version since it has a few significant changes.

Sure, I have put up the PR.
Will just need to confirm if this should be tagged as v2.0.0 given that we already have v1.0.0 in the markbind-action?
(so that I can change the readme documentation to use uses: MarkBind/[email protected] or other tag instead of the current uses: MarkBind/markbind-action@master

[ang-zeyu]

Will just need to confirm if this should be tagged as v2.0.0 given that we already have v1.0.0 in the markbind-action?

yup, sounds good.

Let's add some brief instructions in the devGuide on how to setup a fork for testing / developing this workflow too (no need to go too much into details on surge config, etc.)

[tlylt]

Hi all, I have published a release branch of markbind-action, and now going through one last round of user testing.

The details of how to use the action is available here in the repo's readme.

@MarkBind/active-devs Would like to request help in testing it before we do an official v2 release.

You may follow the examples in the readme and just remember to change the version from v2 to release/v2. So change the uses: MarkBind/markbind-action@v2 below to uses: MarkBind/markbind-action@release/v2 instead

name: MarkBind Action

on:
  push:
    branches:
      - master

jobs: 
  build_and_deploy:
    runs-on: ubuntu-latest
    steps:
      - name: Build & Deploy MarkBind site
        uses: MarkBind/markbind-action@v2
        with:
          token: ${{ secrets.GITHUB_TOKEN }}

to

name: MarkBind Action

on:
  push:
    branches:
      - master

jobs: 
  build_and_deploy:
    runs-on: ubuntu-latest
    steps:
      - name: Build & Deploy MarkBind site
        uses: MarkBind/markbind-action@release/v2
        with:
          token: ${{ secrets.GITHUB_TOKEN }}

Thank you in advance!

[kaixin-hc]

I haven't tested the workflow yet, but i think this part of the readme can be improved by switching the words keep and don't keep!

Keep
* false
* This is the default value

Don't keep
* true
* This will preserve any existing files in the published branch before an update is made.

Thanks @tlylt ! It looks good from reading through.

[jonahtanjz]

Thanks @tlylt! I've tested the main functionalities and they seem to work well 🎉

A couple of comments on the documentation.

[jovyntls]

Thank you for the work @tlylt !

I left some comments on my confusions as a first-time user of Github actions, though if the docs are targeting developers with more experience with Github actions, some suggestions may not be as relevant. Feel free to ignore those not relevant to the target audience 😅

[jovyntls]

Just tried the Github action, this is a very convenient and sleek improvement! Thanks @tlylt

Additional comments after trying out the Github action:

• Tested with an fresh site; the action runs successfully, though I got a 404 error (see https://jovyntls.github.io/sample-mb), though the gh-pages branch is created
• Tried re-running the action and then pushing a new commit to trigger it again. Both times the action runs successfully and the logs don't look suspicious?

If replication is needed: My test repo is here

[kaixin-hc]

Works as described, thanks for doing this! I tested on an existing repo by replacing the github action.

I left one comment and would like to second Jovyn's comments about clarifying the developer facing documentation as well - it's okay, but I think it will be even better if you clarify as she suggested

[tlylt]

Just tried the Github action, this is a very convenient and sleek improvement! Thanks @tlylt

Additional comments after trying out the Github action:

• Tested with an fresh site; the action runs successfully, though I got a 404 error (see jovyntls.github.io/sample-mb), though the gh-pages branch is created
• Tried re-running the action and then pushing a new commit to trigger it again. Both times the action runs successfully and the logs don't look suspicious?

If replication is needed: My test repo is here

Hi @jovyntls, thanks for helping to test this action. The issue could be 1. you did not specify 'baseUrl' in site.json nor in the action workflow. 2. have you configured the settings to serve the site from gh-pages branch?

I think this might have been overlooked but for typical users, these points are mentioned in the UG's deploy site section. I believe the markbind-action's readme also included some reminders on this, but perhaps not enough

[tlylt]

Thank you for the work @tlylt !

I left some comments on my confusions as a first-time user of Github actions, though if the docs are targeting developers with more experience with Github actions, some suggestions may not be as relevant. Feel free to ignore those not relevant to the target audience 😅

I will reword the docs a little. The intention of this section is to give some guidance/instructions for new developers who intend to work on bug fixes/improvements of the markbind-action (and subsequently the reusuable workflows). So as mentioned in the docs, the developer is expected to know the basics of GitHub Actions in order to work it.

[ang-zeyu]

Just dropping in with some very minor formatting nits, the content mostly looks good from my end 👍:

[jonahtanjz]

LGTM from my side 👍

[jovyntls]

Hi @jovyntls, thanks for helping to test this action. The issue could be 1. you did not specify 'baseUrl' in site.json nor in the action workflow. 2. have you configured the settings to serve the site from gh-pages branch?

I think this might have been overlooked but for typical users, these points are mentioned in the UG's deploy site section. I believe the markbind-action's readme also included some reminders on this, but perhaps not enough

I see, my bad! I thought the markbind-action was to automate this, but I realise I misunderstood markbind-action's purpose.

In that case it works as described for me! The docs LGTM; I'll leave approval to anyone more familiar with the DevOps side.

[tlylt]

I see, my bad! I thought the markbind-action was to automate this, but I realise I misunderstood markbind-action's purpose.

Yup, that could be an improvement for the action in the future, such that if the deployment service is gh-pages, we can default to the repository's name.

Thanks everyone for the help!
I have published v2 of the markbind-action and if you would like, you can refer to the action via MarkBind/markbind-action@v2 from now on (This tag will point to the latest v2.*.* version of the action)

[jonahtanjz]

@kaixin-hc You can try to merge this PR :) Do note that we use the squash-merge strategy here to keep the commits atomic in the master branch. You can refer to the managing PR guide.