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

[Feature] Add pulumi k8s await functionality #1353

Merged

Conversation

SchahinRohani
Copy link
Contributor

@SchahinRohani SchahinRohani commented Sep 24, 2024

Description

Bump native-cli go dependencies to enable the pulumi k8s await functionality as described here.

Checklist

  • Updated documentation if needed
  • Tests added/amended
  • bazel test //... passes locally
  • PR is contained in a single commit, using git amend see some docs

This change is Reviewable

Copy link
Member

@aaronmondal aaronmondal left a comment

Choose a reason for hiding this comment

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

Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status: 0 of 1 LGTMs obtained, and all files reviewed, and pending CI: Cargo Dev / macos-13, Installation / macos-13, Installation / macos-14, Installation / ubuntu-22.04, Remote / large-ubuntu-22.04, Web Platform Deployment / macos-14, docker-compose-compiles-nativelink (22.04), macos-13, windows-2022 / stable, and 3 discussions need to be resolved


-- commits line 2 at r1:
Let's rename this to make it clear that the major change is the PULUMI_K8S_AWAIT_ALL


native-cli/default.nix line 32 at r1 (raw file):

    homepage = "https://github.com/TraceMachina/nativelink";
    license = licenses.asl20;
    maintainers = [maintainers.aaronmondal maintainers.schahinrohani];

nit: I believe this is case-sensitive. It probably needs to be SchahinRohani like your username on github, but better double-check whether this is actually true.


native-cli/go.sum line 1 at r1 (raw file):

dario.cat/mergo v1.0.0 h1:AGCNq9Evsj31mOgNPcLyXc+4PNABt905YmuqPYYpBWk=

This file looks like it contains the added new dependencies, but didn't remove the old, unused ones. You can clean this up via go mod tidy and/or deleting the go.sum file entirely and regenerating it. You should see the older versions of the updated packages removed.

@SchahinRohani SchahinRohani changed the title Bump native-cli dependencies [Feature] Add pulumi k8s await functionality Sep 24, 2024
Copy link
Contributor Author

@SchahinRohani SchahinRohani left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 1 LGTMs obtained, and 1 of 4 files reviewed, and pending CI: Analyze (python), Bazel Dev / ubuntu-22.04, Cargo Dev / macos-13, Cargo Dev / ubuntu-22.04, Installation / macos-13, Installation / macos-14, Local / ubuntu-22.04, Publish nativelink-worker-init, Publish nativelink-worker-lre-cc, Remote / large-ubuntu-22.04, Web Platform Deployment / macos-14, Web Platform Deployment / ubuntu-24.04, asan / ubuntu-22.04, docker-compose-compiles-nativelink (22.04), integration-tests (22.04), macos-13, ubuntu-20.04 / stable, ubuntu-22.04, ubuntu-22.04 / stable, windows-2022 / stable, and 2 discussions need to be resolved


-- commits line 2 at r1:

Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…

Let's rename this to make it clear that the major change is the PULUMI_K8S_AWAIT_ALL

Done.


native-cli/go.sum line 1 at r1 (raw file):

Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…

This file looks like it contains the added new dependencies, but didn't remove the old, unused ones. You can clean this up via go mod tidy and/or deleting the go.sum file entirely and regenerating it. You should see the older versions of the updated packages removed.

Done.

Copy link
Member

@aaronmondal aaronmondal left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 3 of 3 files at r2, all commit messages.
Reviewable status: :shipit: complete! 1 of 1 LGTMs obtained, and all files reviewed

@aaronmondal aaronmondal merged commit dfe821c into TraceMachina:main Sep 24, 2024
30 checks passed
@SchahinRohani SchahinRohani deleted the bump/native-cli-dependencies branch September 24, 2024 14:38
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