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

Backport cookie splitter #1662

Prev Previous commit
Next Next commit
Checkout 1.3.14 tag
Signed-off-by: Craig Perkins <cwperx@amazon.com>
  • Loading branch information
cwperks authored and jochen-kressin committed Dec 27, 2023
commit 933078162e489dd2a1547bf8c39580a512ff4346
2 changes: 1 addition & 1 deletion .github/actions/install-dashboards/action.yml
Original file line number Diff line number Diff line change
@@ -48,7 +48,7 @@ runs:
- id: branch-switch-if-possible
continue-on-error: true # Defaults onto main if the branch switch doesn't work
if: ${{ steps.osd-version.outputs.osd-version }}
run: git checkout ${{ steps.osd-version.outputs.osd-version }} || git checkout ${{ steps.osd-version.outputs.osd-x-version }}x
run: git checkout 1.3.14 || git checkout ${{ steps.osd-version.outputs.osd-version }} || git checkout ${{ steps.osd-version.outputs.osd-x-version }}x
Copy link
Member

Choose a reason for hiding this comment

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

why hardcode: git checkout 1.3.14?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure, and maybe that one should go. I cherry picked that commit from Craig's PR, but maybe that was more of a PoC? Without it, the tests would not run, although I can't remember what the error was.
@cwperks Do you know if this was a temporary fix, and what the alternative should be?

Copy link
Member

Choose a reason for hiding this comment

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

If I remember correctly, this was added temporarily because the 1.x branch was stale and #1688 was not merged yet. Since the branch was stale, it was trying to check out an older patch release of OSD core so I added this line in.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @DarshitChanpura is the comment you were awaiting addressed above please, just checking? Thank you.

Copy link
Member

Choose a reason for hiding this comment

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

This line should be removed then I think since the branch is not longer stale.

working-directory: ./OpenSearch-Dashboards
shell: bash