Skip to content

Update deploy command to build site before deployment#2218

Merged
lhw-1 merged 11 commits intoMarkBind:masterfrom
lhw-1:534-build-deploy
Mar 24, 2023
Merged

Update deploy command to build site before deployment#2218
lhw-1 merged 11 commits intoMarkBind:masterfrom
lhw-1:534-build-deploy

Conversation

@lhw-1
Copy link
Contributor

@lhw-1 lhw-1 commented Mar 15, 2023

What is the purpose of this pull request?

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

Breaking Change Overview:

The markbind deploy command now builds and generates the site by default, and this automatic build step can be skipped using the newly introduced --no-build flag. This change is to accommodate the common use case consisting of the following steps:

markbind build
markbind deploy

The markbind build step can now be omitted.

It should be noted that if you have been generating your site with specified values for baseUrl or path to siteConfig.json, you should add the --no-build flag to the markbind deploy command to avoid re-building the site with default values.

If this was being done previously:

markbind build --site-config ./otherSiteConfig.json --baseUrl myDir
markbind deploy

It should now be changed to:

markbind build --site-config ./otherSiteConfig.json --baseUrl myDir
markbind deploy --no-build

Overview of changes:

Resolves #534.

The markbind deploy command now builds and generates the site by default, meaning that authors no longer need to run markbind build before markbind deploy as a mandatory step.

This PR also introduces a --no-build flag to the deploy command, so that authors can choose to instead deploy from a site that was generated previously (i.e. the behavior of the deploy command before this PR). If the author has generated the site with specific arguments (e.g. baseUrl), this option gives them the flexibility to use the specific generation for deployment.

The assumption is that the general use case for the two commands would be markbind build and markbind deploy without additional arguments; this PR simply consolidates the two into one, while preserving the functionality of the original markbind deploy if needed.

Anything you'd like to highlight/discuss:

As an extension to this PR, we can possibly explore improving the deploy command to "remember" previous choices, e.g. if the site was built with specific arguments, then the generation process in the next markbind deploy could re-use these choices. More considerations would be needed here to avoid inadvertently complicating the commands. We could also consider allowing the deploy command to take in the same arguments as the build command, but again, further considerations would be needed to take care in not polluting the functionality of the deploy command.

Additionally, I think that we should refactor the cli/index.js file in the near future (as mentioned in #1756 comment) so as to encourage further modularity and easier development / debugging for the CLI commands and processes.

Testing instructions:

Proposed commit message: (wrap lines at 72 characters)

Update `deploy` command to build site before deployment

The `deploy` command deploys without first building the 
latest version of the site.

The burden is placed on the user to always remember to run 
the `build` command before deploying the new site.

Having the `deploy` command build the site by default allows 
users to build the site with new changes and deploy the site 
with a single command.

Let's incorporate the site generation process into the `deploy` 
command, while providing an optional `--no-build` flag to skip 
the site generation as needed.

Generating the site as part of the deployment is preferable 
as users will need to build before deployment in most cases.

Checklist: ☑️

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

@lhw-1 lhw-1 requested a review from a team March 17, 2023 18:31
@lhw-1 lhw-1 changed the title Generate site within the deploy command Update 'deploy' command to build site concurrently Mar 18, 2023
@lhw-1 lhw-1 changed the title Update 'deploy' command to build site concurrently Update deploy command to build site concurrently Mar 19, 2023
Copy link
Contributor

@jovyntls jovyntls left a comment

Choose a reason for hiding this comment

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

LGTM, mainly nits on documentation!
Remember to use the commit message format as well.

@tlylt
Copy link
Contributor

tlylt commented Mar 20, 2023

quick question: is this really "concurrently" programmatically?

if not let's not use this phrasing as it can be confusing.

@lhw-1 lhw-1 changed the title Update deploy command to build site concurrently Update deploy command to build site before deployment Mar 20, 2023
Copy link
Contributor

@jovyntls jovyntls left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the docs additions & detailed commit message @lhw-1 !

When it's ready (by the DG project management guidelines), feel free to try merging this PR! Remember to use the squash-merge strategy with the commit message :)

@jovyntls jovyntls modified the milestones: v4.1.1, v4.2.0, v5.0.0 Mar 23, 2023
@jovyntls jovyntls added the breakingChange 💥 Feature will behave significantly different, or is made obsolete label Mar 23, 2023
@lhw-1 lhw-1 merged commit 9cac124 into MarkBind:master Mar 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breakingChange 💥 Feature will behave significantly different, or is made obsolete

Projects

None yet

Development

Successfully merging this pull request may close these issues.

markbind deploy: build before deploying

4 participants