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

markbind deploy: build before deploying #534

Closed
damithc opened this issue Jan 4, 2019 · 6 comments · Fixed by #2218
Closed

markbind deploy: build before deploying #534

damithc opened this issue Jan 4, 2019 · 6 comments · Fixed by #2218
Assignees
Labels
a-CLI s.UnderDiscussion The team will evaluate this issue to decide whether it is worth adding

Comments

@damithc
Copy link
Contributor

damithc commented Jan 4, 2019

Current: markbind deploy does not build but deploy only.

In most cases users would want to deploy the latest version of the site. Should it build by default with possibly a --no-build option?

@damithc damithc added s.UnderDiscussion The team will evaluate this issue to decide whether it is worth adding a-CLI labels Jan 4, 2019
@yamgent
Copy link
Member

yamgent commented Jan 5, 2019

Commands should usually do only a single thing though. It can have additional features to ease typing, but those features are usually off by default, not on by default.

So a markbind deploy will still deploy only, but if the user wants a build to happen as well, then he should specify a --build option:

markbind deploy --build

This principle is also followed by other command line tools such as git:

git checkout existing-branch  # check out an existing branch (error if branch does not exist)
git checkout -b my-new-branch # create a new branch first from current branch, then check it out

@damithc
Copy link
Contributor Author

damithc commented Jan 5, 2019

Commands should usually do only a single thing though. It can have additional features to ease typing, but those features are usually off by default, not on by default.

That's a good point. I'm Ok with a -b | --build flag.

BTW, isn't git pull command an exception to that principle? i.e., it fetches and merges in one shot.

@yamgent
Copy link
Member

yamgent commented Jan 5, 2019

BTW, isn't git pull command an exception to that principle? i.e., it fetches and merges in one shot.

git's pull is a composed command though, it calls other commands (git fetch <options> and git merge <options>) in order to achieve its objective of updating the branch.

We could also create a brand new command that is composed, just like git's pull. Maybe something like markbind publish, which calls markbind build && markbind deploy? Not sure whether publish is a good name though.

@damithc
Copy link
Contributor Author

damithc commented Jan 5, 2019

We could also create a brand new command that is composed, just like git's pull. Maybe something like markbind publish, which calls markbind build && markbind deploy? Not sure whether publish is a good name though.

Sounds good. Alternatively, we can rename deploy to push or upload and use deploy for the new command.

@lhw-1
Copy link
Contributor

lhw-1 commented Mar 14, 2023

While this issue has been dormant for years, I think this is definitely still worth implementing as it allows for build before deploy.

With regards to having commands only do a single thing, while in principle this is true, our markbind serve command will build and then serve, so this principle is not exactly being followed. Of course, for the serve command this is more of a necessity, but I think that as per the original suggestion, having the deploy command build by default and having a --no-build option could still be on the table if we are already not following this idea strictly.

@damithc What do you think? Personally, at the moment this makes more sense than coming up with a new command altogether that simply combines the two commands. Of course, if you think that coming up with a new command is still better, then I think push could be a good name for it as you suggested (publish might be too similar to deploy).

@lhw-1 lhw-1 self-assigned this Mar 14, 2023
@damithc
Copy link
Contributor Author

damithc commented Mar 15, 2023

@damithc What do you think? Personally, at the moment this makes more sense than coming up with a new command altogether that simply combines the two commands. Of course, if you think that coming up with a new command is still better, then I think push could be a good name for it as you suggested (publish might be too similar to deploy).

@lhw-1 I'm OK with deploy command building by default while providing a --no-build option. A few times I ended up deploying without building properly which resulted in a partial website being deployed.

Other views are welcome too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-CLI s.UnderDiscussion The team will evaluate this issue to decide whether it is worth adding
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants