Update deploy command to build site before deployment#2218
Merged
lhw-1 merged 11 commits intoMarkBind:masterfrom Mar 24, 2023
Merged
Update deploy command to build site before deployment#2218lhw-1 merged 11 commits intoMarkBind:masterfrom
deploy command to build site before deployment#2218lhw-1 merged 11 commits intoMarkBind:masterfrom
Conversation
EltonGohJH
reviewed
Mar 18, 2023
EltonGohJH
reviewed
Mar 18, 2023
EltonGohJH
reviewed
Mar 18, 2023
…into 534-build-deploy
deploy commanddeploy command to build site concurrently
jovyntls
reviewed
Mar 20, 2023
Contributor
|
quick question: is this really "concurrently" programmatically? if not let's not use this phrasing as it can be confusing. |
deploy command to build site concurrentlydeploy command to build site before deployment
jovyntls
approved these changes
Mar 23, 2023
Contributor
jovyntls
left a comment
There was a problem hiding this comment.
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 :)
This was referenced Mar 26, 2023
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What is the purpose of this pull request?
Breaking Change Overview:
The
markbind deploycommand now builds and generates the site by default, and this automatic build step can be skipped using the newly introduced--no-buildflag. This change is to accommodate the common use case consisting of the following steps:The
markbind buildstep can now be omitted.It should be noted that if you have been generating your site with specified values for
baseUrlor path tositeConfig.json, you should add the--no-buildflag to themarkbind deploycommand to avoid re-building the site with default values.Overview of changes:
Resolves #534.
The
markbind deploycommand now builds and generates the site by default, meaning that authors no longer need to runmarkbind buildbeforemarkbind deployas a mandatory step.This PR also introduces a
--no-buildflag to thedeploycommand, so that authors can choose to instead deploy from a site that was generated previously (i.e. the behavior of thedeploycommand 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 buildandmarkbind deploywithout additional arguments; this PR simply consolidates the two into one, while preserving the functionality of the originalmarkbind deployif needed.Anything you'd like to highlight/discuss:
As an extension to this PR, we can possibly explore improving the
deploycommand to "remember" previous choices, e.g. if the site was built with specific arguments, then the generation process in the nextmarkbind deploycould re-use these choices. More considerations would be needed here to avoid inadvertently complicating the commands. We could also consider allowing thedeploycommand to take in the same arguments as thebuildcommand, but again, further considerations would be needed to take care in not polluting the functionality of thedeploycommand.Additionally, I think that we should refactor the
cli/index.jsfile 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)
Checklist: ☑️