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

[Bug]: Version bump PR for functional test repo failed to update pack-lock.json #4638

Open
zelinh opened this issue Apr 12, 2024 · 6 comments
Labels
bug Something isn't working

Comments

@zelinh
Copy link
Member

zelinh commented Apr 12, 2024

Describe the bug

The auto-generated version bump PR for functional test repo didn't include package-lock.json file.
e.g. opensearch-project/opensearch-dashboards-functional-test#1216

It should have increment the version in package-lock.json here https://github.com/opensearch-project/opensearch-dashboards-functional-test/blob/45396abd554474e1eca18f20bb77bf42ee11fafa/package-lock.json#L3
along with package.json.

Seems like we missed it here.

elif [ ${{ matrix.entry.repo }} == "opensearch-dashboards-functional-test" ]; then
jq --arg DASHBOARD_VERSION "${{ env.DASHBOARD_VERSION }}" '.version = $DASHBOARD_VERSION' package.json > package-tmp.json
mv package-tmp.json package.json
OSD_PLUGIN_VERSION=$(node -p "require('./package.json').version")
echo "OSD_PLUGIN_VERSION=$OSD_PLUGIN_VERSION" >> $GITHUB_ENV

To reproduce

Run the current GHA Increment OpenSearch Dashboards Plugins Version

Expected behavior

All two json files should have version incremented.

@zelinh zelinh added bug Something isn't working untriaged Issues that have not yet been triaged labels Apr 12, 2024
@prudhvigodithi
Copy link
Collaborator

Hey @zelinh that is done on purpose as AFAIK for FT repo we dont package-lock.json file, here are some sample old PR's that only increments package.json
https://github.com/opensearch-project/opensearch-dashboards-functional-test/pull/1206/files
opensearch-project/opensearch-dashboards-functional-test#1121
Adding @AMoo-Miki to confirm.

@bbarani @Divyaasm

@Divyaasm Divyaasm removed the untriaged Issues that have not yet been triaged label Apr 23, 2024
@AMoo-Miki
Copy link
Contributor

Yeah; we shouldn't need to bump it but it could help with tracking if we ever need to do so (which we have never needed to).

@rishabh6788
Copy link
Collaborator

Do we have a consensus on whether it should or should not be included? @AMoo-Miki

@AMoo-Miki
Copy link
Contributor

Thinking about it more, we should keep the versions in the lock file and the manifest in sync.

@prudhvigodithi
Copy link
Collaborator

There is some discussion in past #3856 related to yarn.lock file. If just adding package-lock.json is required then:

Thank you
@peterzhuamazon @getsaurabh02

@Divyaasm
Copy link
Collaborator

The package-lock.json is not being updated currently check here. Do we still need to automate this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Backlog
Development

No branches or pull requests

5 participants