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(action)!: Fix preserve-block-position default #393

Merged
merged 1 commit into from
Jan 15, 2024

Conversation

bmaximuml
Copy link
Contributor

Current logic adds --preserve-block-position to list of args if the length of the value provided to the preserve-block-position github actions variable is not zero. Since it defaults to the string false, this effectively defaults it to true. You can only disable it by setting it explicitly to an empty string. This PR corrects this logic by only enabling preserve-block-position if it's explicitly set to true.

BREAKING CHANGE: Effectively change default value for preserve-block-position in github actions

Current logic adds `--preserve-block-position` to list of args if the length of
the value provided to the `preserve-block-position` github actions variable is
not zero. Since it defaults to the string `false`, this effectively defaults it
to true. You can only disable it by setting it explicitly to an empty string.
This PR corrects this logic by only enabling `preserve-block-position` if it's
explicitly set to true.

BREAKING CHANGE: Effectively change default value for preserve-block-position
in github actions
@bmaximuml
Copy link
Contributor Author

@gagoar please could you take a look at this?

@coveralls
Copy link
Collaborator

Pull Request Test Coverage Report for Build 7412441122

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 100.0%

Totals Coverage Status
Change from base Build 6989677130: 0.0%
Covered Lines: 263
Relevant Lines: 263

💛 - Coveralls

@gagoar
Copy link
Owner

gagoar commented Jan 10, 2024

@bmaximuml Thanks for this; I will release the action and the other PR on Friday.

Copy link

codecov bot commented Jan 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (91bab21) 100.00% compared to head (236fa43) 100.00%.

Additional details and impacted files
@@            Coverage Diff            @@
##            master      #393   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           11        11           
  Lines          284       284           
  Branches        64        64           
=========================================
  Hits           284       284           
Flag Coverage Δ
unittests 100.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@bmaximuml
Copy link
Contributor Author

Fantastic! Thanks @gagoar

@gagoar
Copy link
Owner

gagoar commented Jan 10, 2024

Fantastic! Thanks @gagoar

thank you @bmaximuml ❤️ and sorry for the delay.

@gustavkj
Copy link
Collaborator

@gagoar Since the Github Action still is release manually, I'm not sure if this should be marked as breaking since that will only cause the npm library to update to a new major version. 🤔

@gagoar
Copy link
Owner

gagoar commented Jan 12, 2024

@gagoar Since the Github Action still is release manually, I'm not sure if this should be marked as breaking since that will only cause the npm library to update to a new major version. 🤔

Yea. What I would do is:

@gustavkj
Copy link
Collaborator

I've released the changes now by merging the release-please PR (I had to push a commit of my own to it to make the workflows run).

Another option to the versioning would be to use the same version numbers. The drawback is that breaking changes to either would result in a major version bump.

@gagoar
Copy link
Owner

gagoar commented Jan 12, 2024

I've released the changes now by merging the release-please PR (I had to push a commit of my own to it to make the workflows run).

Another option to the versioning would be to use the same version numbers. The drawback is that breaking changes to either would result in a major version bump.

I chose not to do that when I started with the action. The reason is the nature of the code:

  • The action advances only when the CLI changes. I can even make that more automatic in the future.
  • The library might have bugs or improvements unrelated to the CLI.

@gagoar gagoar merged commit f4ae6ee into gagoar:master Jan 15, 2024
10 checks passed
@gagoar
Copy link
Owner

gagoar commented Jan 15, 2024

hey @bmaximuml here's the release https://github.com/gagoar/codeowners-generator/releases/tag/action.v2.0

thanks for your help!

@bmaximuml bmaximuml deleted the bug-preserve-block-position branch January 24, 2024 08:57
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.

4 participants