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

Fix jq version for github actions #334

Merged
merged 6 commits into from
Mar 27, 2024
Merged

Fix jq version for github actions #334

merged 6 commits into from
Mar 27, 2024

Conversation

Kidswiss
Copy link
Contributor

@Kidswiss Kidswiss commented Mar 26, 2024

Checklist

  • The PR has a meaningful title. It will be used to auto generate the
    changelog.
    The PR has a meaningful description that sums up the change. It will be
    linked in the changelog.
  • PR contains a single logical change (to build a better changelog).
  • Categorize the PR by adding one of the labels:
    bug, enhancement, documentation, change, breaking, dependency
    as they show up in the changelog.
  • Link this PR to related issues or PRs.

@Kidswiss Kidswiss added the bug Something isn't working label Mar 26, 2024
@Kidswiss Kidswiss force-pushed the fix/e2e-jq branch 10 times, most recently from 1c69467 to 1df1520 Compare March 27, 2024 12:28
@Kidswiss Kidswiss force-pushed the fix/e2e-jq branch 2 times, most recently from 3b4e44f to d3db2d9 Compare March 27, 2024 13:05
@Kidswiss Kidswiss requested review from a team, TheBigLee, wejdross and zugao and removed request for a team March 27, 2024 13:05
],
kind: 'Secret',
name: 'github-ci-secret',
namespace: 'schedar-e2e',
Copy link
Contributor

Choose a reason for hiding this comment

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

I would keep the project name instead of team name.

Copy link
Member

@wejdross wejdross Mar 27, 2024

Choose a reason for hiding this comment

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

I wouldn't - it's descriptive for all engineers right now, it's also easy way to contact with us.
Component can be owned by different teams, schedar is just one

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's currently deployed as schedar-e2e, this adjustment makes that the jsonnet actually matches what's deployed.

But if you like the old name more, I can change it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wouldn't - it's descriptive for all engineers right now, it's also easy way to contact with us. Component can be owned by different teams, schedar is just one

True, but schedar owns appcat, so it should be the same either way.

But thinking a bit more about the naming, it might make sense to go with appcat-e2e. Otherwise, we're leaking some vshn internal naming to the product.

@Kidswiss Kidswiss requested a review from zugao March 27, 2024 13:10
Copy link
Member

@wejdross wejdross left a comment

Choose a reason for hiding this comment

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

looks good, I saw it worked for LAB cluster, I don't care that much about namespace name, LGTM

@Kidswiss Kidswiss merged commit 485bcaa into master Mar 27, 2024
31 of 32 checks passed
@Kidswiss Kidswiss deleted the fix/e2e-jq branch March 27, 2024 14:06
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
None yet
Development

Successfully merging this pull request may close these issues.

3 participants