opensearch-project / opensearch-build

🧰 OpenSearch / OpenSearch-Dashboards Build Systems
Apache License 2.0
137 stars 272 forks source link

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

Open zelinh opened 6 months ago

zelinh commented 6 months ago

Describe the bug

The auto-generated version bump PR for functional test repo didn't include package-lock.json file. e.g. https://github.com/opensearch-project/opensearch-dashboards-functional-test/pull/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. https://github.com/opensearch-project/opensearch-build/blob/90352da641556ad9c900e8a5f9149fd836556319/.github/workflows/osd-increment-plugin-versions.yml#L95-L99

To reproduce

Run the current GHA Increment OpenSearch Dashboards Plugins Version

Expected behavior

All two json files should have version incremented.

prudhvigodithi commented 5 months ago

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 https://github.com/opensearch-project/opensearch-dashboards-functional-test/pull/1121 Adding @AMoo-Miki to confirm.

@bbarani @Divyaasm

AMoo-Miki commented 5 months ago

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 commented 3 months ago

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

AMoo-Miki commented 3 days ago

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

prudhvigodithi commented 2 days ago

There is some discussion in past https://github.com/opensearch-project/opensearch-build/issues/3856 related to yarn.lock file. If just adding package-lock.json is required then:

Thank you @peterzhuamazon @getsaurabh02