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

🚧 enhance get current version #117

Merged
merged 11 commits into from
Mar 23, 2021
Merged

🚧 enhance get current version #117

merged 11 commits into from
Mar 23, 2021

Conversation

edbzn
Copy link
Member

@edbzn edbzn commented Mar 11, 2021

Following this discussion #102, handle the current version differently, @yjaaidi what do you think?

@codecov
Copy link

codecov bot commented Mar 12, 2021

Codecov Report

Merging #117 (88d51d2) into main (aa1672c) will decrease coverage by 0.19%.
The diff coverage is 98.07%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #117      +/-   ##
==========================================
- Coverage   96.15%   95.96%   -0.20%     
==========================================
  Files          16       16              
  Lines         312      322      +10     
  Branches       35       32       -3     
==========================================
+ Hits          300      309       +9     
- Misses          7        8       +1     
  Partials        5        5              
Impacted Files Coverage Δ
...kages/semver/src/builders/version/utils/project.ts 100.00% <ø> (ø)
packages/semver/src/builders/version/testing.ts 95.65% <66.66%> (-4.35%) ⬇️
packages/semver/src/builders/version/builder.ts 97.22% <100.00%> (ø)
...ver/src/builders/version/utils/get-last-version.ts 100.00% <100.00%> (ø)
packages/semver/src/builders/version/utils/git.ts 87.09% <100.00%> (+1.38%) ⬆️
...ages/semver/src/builders/version/utils/try-bump.ts 100.00% <100.00%> (ø)
packages/semver/src/builders/version/version.ts 100.00% <100.00%> (ø)

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 aa1672c...88d51d2. Read the comment docs.

@edbzn edbzn requested a review from yjaaidi March 19, 2021 07:59
@edbzn edbzn added the enhancement New feature or request label Mar 19, 2021
Copy link
Member

@yjaaidi yjaaidi left a comment

Choose a reason for hiding this comment

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

As mentioned below, it's probably better to avoid the last tag detection fallback.
We should warn the user that we'll be using the initial commit as a base.

What do you think @edbzn?

Comment on lines 43 to 47
getLastTag().pipe(
tap((tag) => {
logger.warn(`🟠 No previous semver tag found, fallback from: ${tag}`);
})
)
Copy link
Member

Choose a reason for hiding this comment

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

@edbzn I think that getLastTag is a dangerous fallback heuristic. In most cases, this will produce a surprising behavior.
I am thinking about a monorepo with lots of already versioned packages (a=3.0.0, b=4.0.0)then we add a new package. This package will then use the version of the last tagged package (a or b) which is wrong.

we can just skip this step

Copy link
Member

Choose a reason for hiding this comment

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

The other thing here is that getLastTag() returns a tag and not a version.

Comment on lines 42 to 50
switchMap((since) =>
iif(() => since === `0.0.0`, getFirstCommitRef(), of(since))
)
Copy link
Member

Choose a reason for hiding this comment

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

When the current version is not found, maybe since's value should be null. This way we can make the difference between detecting a 0.0.0 version and not detecting any version.

If since is null (or currently 0.0.0), then before bumping, we can log something like: Couldn't find a previous version tag for this project. New version will be calculated based on all changes since first commit. If your project is already versioned, please tag the last release commit with my-prefix-x-y-z and run this command again.

@edbzn
Copy link
Member Author

edbzn commented Mar 23, 2021

Agree! Let's improve this.

@edbzn edbzn merged commit 7cf4507 into main Mar 23, 2021
@edbzn edbzn deleted the fix/git-ref branch March 23, 2021 16:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants