Skip to content

chore: corrected currency update logic to prevent mixing major and minor update#2464

Merged
aryamohanan merged 10 commits intomainfrom
chore-currency-update-bug
Apr 7, 2026
Merged

chore: corrected currency update logic to prevent mixing major and minor update#2464
aryamohanan merged 10 commits intomainfrom
chore-currency-update-bug

Conversation

@aryamohanan
Copy link
Copy Markdown
Contributor

@aryamohanan aryamohanan commented Apr 6, 2026

There are a couple of issues:

  1. When a major update is applied, minor updates are also being included.
  2. When multiple major updates occur, it additionally includes the major version updates from packages in the previously created PR, along with other minor updates.

see this PR #2461

@aryamohanan aryamohanan force-pushed the chore-currency-update-bug branch from af005e8 to 7cfae62 Compare April 6, 2026 10:00
@abhilash-sivan abhilash-sivan self-requested a review April 6, 2026 12:56
@aryamohanan aryamohanan force-pushed the chore-currency-update-bug branch from 3b3e1b5 to 02a7fc4 Compare April 6, 2026 13:23
@abhilash-sivan
Copy link
Copy Markdown
Contributor

test results:

case 1: currency-bot branch exists
case2: currency-bot does not exists

Case 1:
The major version is being added to the version list, which I don’t think is necessary. If a breaking change requires supporting multiple major versions, we can handle that explicitly when needed.
Also, this change aggregates all major updates and is applied across all branches.
ie, test-3-ibmdb-400 an example branch which should only consist of ibm db changes does include both IBM DB, GOT, etc. (all major updates together)

Case 2:
Looks good — the major version is replaced instead. This aligns with what we likely need.
 
IMO we need case 2 result in the both scenarios

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Apr 6, 2026

@aryamohanan aryamohanan merged commit 1d9eccf into main Apr 7, 2026
1 check was pending

console.log(`Latest version: ${latestVersion}`);
console.log(`Installed version: ${installedVersion}`);
const isMajorUpdate = semver.major(latestVersion) !== semver.major(installedVersion);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This condition was wrong earier

if (MAJOR_UPDATES_MODE) {
utils.prepareGitEnvironment('main', cwd, true, DRY_RUN);

currencies = loadCurrencies();
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We load the currencies file fresh, else it will load from the altered one.

typeof installedVersionObj === 'string' ? latestVersion : { ...installedVersionObj, v: latestVersion };
currency.versions.splice(installedVersionIndex, 0, newVersionObj);
}
const newVersionObj =
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

In both minor and major update we replace the latest current version in the version list.

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.

2 participants