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

[hlx deploy] must run hlx build #241

Closed
kptdobe opened this issue Oct 4, 2018 · 7 comments · Fixed by #1064
Closed

[hlx deploy] must run hlx build #241

kptdobe opened this issue Oct 4, 2018 · 7 comments · Fixed by #1064
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers released

Comments

@kptdobe
Copy link
Contributor

kptdobe commented Oct 4, 2018

Is your feature request related to a problem? Please describe.
While setting up my production environment and fixing some bugs, I had to run deploy multiple times. Even though I test with hlx up before running the hlx deploy, the build is not up-to-date and I keep forgetting to run hlx build. Testing on production is pretty slow, thus I keep losing time understanding why my changes did not have the expected result. The only reason is that the code is not live... Error prone and quite frequent.

Describe the solution you'd like
hlx deploy should either run a hlx build for me. Telling me the build is not up to date would not be a good option because I never want to deploy an older version... (I could still checkout the old code if really needed)

Describe alternatives you've considered
N/A

@kptdobe kptdobe added the enhancement New feature or request label Oct 4, 2018
@tripodsan
Copy link
Contributor

hlx up does exactly the same thing as helix build. so I don't understand why this makes a difference.

but I agree. I think we can add a forced build before each deploy. maybe with an extra flag to disable if one like: --no-build

@tripodsan tripodsan added the good first issue Good for newcomers label Oct 5, 2018
@trieloff
Copy link
Contributor

trieloff commented Oct 8, 2018

Should we also add a forced deploy before strain?

@tripodsan
Copy link
Contributor

Should we also add a forced deploy before strain?

no. strains and actions are not related.

@tripodsan
Copy link
Contributor

@lkrapf had this issue, too....

@bdelacretaz
Copy link

I was also hit bit this today in adobe/helix-test-content-onedrive-auth#2 - pinging the developer experience theme, adobe/helix-home#40 to link this issue there.

@rofe
Copy link
Contributor

rofe commented Jul 4, 2019

I understand the goal here is that developers shouldn't have to remember to run hlx build. I looked into the code a bit, and I think instead of triggering build from deploy and adding a new argument to control it there, we should actually have package trigger build:

package should probably never run without (or with outdated) scripts anyway. And deploy already supports an argument determining if packages should be (re)built before deploying. I think it would make sense to piggy-back on this argument instead of adding a potentially conflicting/nonsensical --no-build. So when running deploy, build would always be triggered implicitly through package, unless you call hlx deploy --package=ignore.

If we should ever feel that developers must still be able to avoid (re)building for whatever reason, we could e.g.:

  • add a --no-build argument to package, and some 4th option to deploy --package=..., or
  • keep a timestamp of last package creation and skip the build step if the last modified dates of scripts are older

rofe added a commit that referenced this issue Jul 5, 2019
@rofe rofe closed this as completed in #1064 Jul 8, 2019
rofe added a commit that referenced this issue Jul 8, 2019
trieloff pushed a commit that referenced this issue Jul 8, 2019
# [4.9.0](v4.8.1...v4.9.0) (2019-07-08)

### Features

* **deploy:** must run hlx build ([#241](#241)) ([3981455](3981455))
@adobe-bot
Copy link
Collaborator

🎉 This issue has been resolved in version 4.9.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers released
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants