Skip to content

bug: Fix bumping error #198

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

Merged
merged 4 commits into from
May 5, 2021
Merged

bug: Fix bumping error #198

merged 4 commits into from
May 5, 2021

Conversation

minkimcello
Copy link
Collaborator

@minkimcello minkimcello commented May 5, 2021

Motivation

I noticed a bug in covector on how it's supposed to bump inter-dependent packages. It wasn't showing up in tests but in the covector setup within the covector monorepo workspace so this PR is meant to locate why that was happening.

Approach

  • Before I get into how I fixed the bug, I found another bug from one of my refactors in feat: add preview command to covector #187. I had forgotten to rename one of the argument variables and that's been addressed.
  • I also accidentally deleted a fixture file so that was reintroduced in this PR
  • The bug bug came from the covector config file. The packages listed in the config file must match the package names listed in the packages' package.json files. Fixing this allowed covector to properly bump all the packages and dependencies. See data below in comments.

@minkimcello minkimcello changed the base branch from main to mk/run-preview May 5, 2021 14:04
@minkimcello
Copy link
Collaborator Author

minkimcello commented May 5, 2021

action run 2509947803

change files

action, apply, assemble, changelog, command, covector, files (all)

expected

every package and its covector dependencies to be bumped to the preview version

received

apply: only package version is bumped, assemble and files dependencies are not bumped
assemble: only package version is bumped, command and files dependencies are not bumped
changelog: only package version is bumped, files dependency is not bumped
command: only package version is bumped, assemble devDependency not bumped but expected
covector: only package is bumped, apply, assemble, changelog, command, files not bumped
files: package is bumped and has no covector-related dependencies
action: package not bumped as expected

It's a wild guess but this might be happening because the config's package names do not match the actual package versions as they're listed in the dependencies

@minkimcello
Copy link
Collaborator Author

action run 2510619670

change files

action, apply, assemble, changelog, command, covector, files (all)

expected

every package and its covector dependencies to be bumped to the preview version

received

everything being bumped as it's supposed to. the fix was to update the config file because i guess the packages specified in config are matched to the names listed in the packages' packge.json.

@minkimcello
Copy link
Collaborator Author

minkimcello commented May 5, 2021

action run 2510799442

change files

just assemble.

expected

covector, apply, and command depends on assemble so we can expect those packages to be bumped with its dependency of assemble bumped too.

received

covector: bumped and apply, command, assembled bumped
command: bumped and assembled bumped
assemble: bumped and command bumped
apply: bumped and assembled bumped

👍

@minkimcello minkimcello changed the title [WIP] Locating Bumping Bug bug: Fix bumping error May 5, 2021
@minkimcello minkimcello requested a review from jbolda May 5, 2021 16:24
@minkimcello minkimcello merged commit a0a3f5f into mk/run-preview May 5, 2021
@minkimcello minkimcello deleted the mk/bug-locate branch May 5, 2021 17:33
minkimcello added a commit that referenced this pull request May 5, 2021

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
* Disable workflows and update location of preview action

* Specify label and previewVersion inputs for action manifest

* Add conditional for version bumping for previews

* Add versioning steps for preview command

* Pass in context and previewVersion into covector from action

* Add publishing steps for preview command

* Run action locally for preview workflow

* Remove post publish commands from preview

* Double timeout length for preview command

* Re-enable the other workflows

* Remove unused argument in apply

* Modify change file to include apply and covector

* Add preview label to version PR creator

* Undo doubling timeout for preview

* Update invalid template error message

* Throw error when preview runs on non-pr

* Add action argument for branch identifier

* Change change files to minor bump

* Remove unnecessary argument

* Fix apply bug for bumpDep

* Add test for preview apply

* Add test for preview covector

* Remove obsolete snapshots

* Add console log for preview version bumps

* Update snapshot

* New fixture for preview

* New new fixture

* Fix rust bug

* Make test use new fixture

* bug: Fix bumping error (#198)

* Fix overlooked refactor of a variable in apply

* Rename package files in config to match the packages json files

* Reintroduce accidentally deleted toml file in fixtures

* Fix change files with the correct package names
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.

None yet

2 participants