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

Add fold option to avoid version code jumps such as v1.2.3 -> v8.6.3 #45

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

FelixZY
Copy link

@FelixZY FelixZY commented Jan 3, 2024

I've used semantic-release before where each version increment is quite minimal.

E.g.

fix: lorem ipsum
feat: lorem ipsum
fix: lorem ipsum

would result in a version bump to major.minor+1.0 rather than major.minor+1.patch+1, which is the behavior of this repo. This issue was discussed in #11 where @xotahal welcomed contributions - so here it is 😉 .

#11 also discusses support for "locked" version numbers. However, this is not something I have implemented at this time as I think it warrants further discussion first. Personally, I think the behavior of semantic-release should be imitated.

This makes it easier to make manual changes to the original version number.
@FelixZY
Copy link
Author

FelixZY commented Jan 4, 2024

I did NOT read the code closely enough when making these changes. Working on an actually working version.

…ruct

This makes version codes easier to work with and consider a single entity.
This looks like an oversight which could result in
`RELEASE_IS_NEXT_VERSION_COMPATIBLE_WITH_CODEPUSH`
erroneously reporting true or false when compared with
`RELEASE_LAST_INCOMPATIBLE_CODEPUSH_VERSION`
…vidual responsibility clearer

`is_releasable` and `is_codepush_friendly` does more and other things than their names suggest.
Primarily, they modify the `lane_context` (unexpected side effects) but `is_codepush_friendly` also
does not actually answer this question.

This commit renames `is_releasable` to `calculate_versions` and
`is_codepush_friendly` to `calculate_last_codepush_incompatible_version`. The commit also extracts
`lane_context` modifications to a separate method.

This commit does cause slight breakage in that the
`RELEASE_LAST_INCOMPATIBLE_CODEPUSH_VERSION` context variable will not be set if
`get_beginning_of_next_sprint` fails. However, I believe this to be an error state - which should probably
throw or something - anyways and have thus ignored this slight behavioral change.
@FelixZY FelixZY force-pushed the master branch 3 times, most recently from fab81ec to a26b387 Compare January 4, 2024 22:56
This reduces code duplication and is more expected from a method "get commits".

This change also applies the `SemanticReleaseHelper.should_exclude_commit`
filter for `calculate_last_codepush_incompatible_version`. I do not know why this
filter was not applied here previously but assume its omission to be a bug.
This will be used to optimize `calculate_last_codepush_incompatible_version` and
reduce code duplication.
…lculate_commit_version`

This commit primarily aims to reduce code duplication, improve readability and enforce a stricter
"one task per method" mindset. The commit also aims to optimize the codepush incompatible
commit calculation.

The commit attempts to achieve these goals by:
* Improved naming
* Explicit parameters rather than `params`
* A centralized `calculate_commit_version` method
* Working from the most recent commits rather than the oldest when calculating codepush
  incompatible commits
* introducing the ability to retrieve the previous release for *any* commit, not just the most
  recent one

I have tried to retain backwards-compatibility from a user standpoint (i.e. the program
behaves the same as it did before). However, a few log messages were not possible to include
in a simple way without incurring log-specific overhead:
* "Found a tag #{tag_name} associated with version #{version}" - would either print multiple
  times or not at all. I opted for the latter.
* "Found #{commits.length} commits since last release" - I'm using lazy iterators and therefore do
  not know the number of commits beforehand. Also, due to code deduplication, this would now
  print multiple times. For these reasons, I opted for removal.
As compensation, I added two new log prints:
* "Last version: #{last_version}"
* "Next version: #{next_version}"
As noted in xotahal#11, for some users that are used to
[semantic-release](xotahal#11)
it is quite unexpected that the version number increments _for each commit_ rather than
_per changeset_.

This commit introduces the more expected behavior as a non-breaking change via the optional
`fold` parameter.

I have not added new tests at this time as implementing this has taken
way longer and grown more complex than I intended.
@FelixZY
Copy link
Author

FelixZY commented Jan 4, 2024

Now it's fixed. As noted in my last commit, I have not added tests for the fold behavior. This is partly due to me not intending to spend this amount of time to start with and partly due to me not being a ruby dev - this is my first "real" ruby work 😅 . I'm not sure what to do about the remaining failing checks in circle ci; I won't claim (knowing) responsibility for issues such as these 🙂 :

lib/fastlane/plugin/semantic_release/actions/analyze_commits.rb:4:1: E: module definition in method body
(Using Ruby 2.0 parser; configure using TargetRubyVersion parameter, under AllCops)
module Fastlane
^^^^^^

…und `calculate_commit_version`

Fix invalid params reference
…und `calculate_commit_version`

Fix invalid backup return (expected 2 values, got 1)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant