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

fix(cli): add error message for hlx up command in submodule repo #1796

Merged
merged 2 commits into from
Jul 1, 2021

Conversation

maxakuru
Copy link
Member

@maxakuru maxakuru commented Jul 1, 2021

Related Issues

#1794

@codecov
Copy link

codecov bot commented Jul 1, 2021

Codecov Report

Merging #1796 (94665a8) into main (289b590) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1796      +/-   ##
==========================================
+ Coverage   94.79%   94.80%   +0.01%     
==========================================
  Files          35       35              
  Lines        1748     1753       +5     
==========================================
+ Hits         1657     1662       +5     
  Misses         91       91              
Impacted Files Coverage Δ
src/up.cmd.js 88.11% <100.00%> (+0.30%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 636cda0...94665a8. Read the comment docs.

src/up.cmd.js Outdated
if (!await fse.pathExists(path.join(this.directory, '.git'))) {
throw Error('hlx up needs local git repository.');
try {
const stat = await fse.lstat(path.join(this.directory, '.git'));
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const stat = await fse.lstat(path.join(this.directory, '.git'));
const stat = await fse.lstat(path.resolve(this.directory, '.git'));

tripodsan
tripodsan previously approved these changes Jul 1, 2021
Copy link
Contributor

@tripodsan tripodsan left a comment

Choose a reason for hiding this comment

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

I would use path.resolve(). otherwise, LGTM.

@maxakuru
Copy link
Member Author

maxakuru commented Jul 1, 2021

@tripodsan what's the process for merges? squash? let approver merge?

@tripodsan
Copy link
Contributor

@tripodsan what's the process for merges? squash? let approver merge?

no, it's usually the PR author that does the merge. if squash or merge is a personal choice :-) but all commits need to adhere to the commit message rules (https://github.com/conventional-changelog/conventional-changelog/tree/master/packages/conventional-changelog-angular)

so if you have a lot for small (wip) commits, you should either squash them before the merge, or do a sqash commit in github. but if you have a nice set of commits, feel free to just merge them.

note, that semantic-release will create a released base on the commit types. so if you have a fix: ... commit in the history, if will create a release, of all the commits are only chore: it will not create a release.

@maxakuru maxakuru merged commit 835f80a into adobe:main Jul 1, 2021
adobe-bot pushed a commit that referenced this pull request Jul 1, 2021
## [13.4.50](v13.4.49...v13.4.50) (2021-07-01)

### Bug Fixes

* **cli:** add error message for hlx up command in submodule repo ([#1796](#1796)) ([835f80a](835f80a)), closes [#1794](#1794)
@adobe-bot
Copy link
Collaborator

🎉 This PR is included in version 13.4.50 🎉

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants