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

chore: remove codersdk dependency #194

Merged
merged 6 commits into from
May 17, 2024
Merged

chore: remove codersdk dependency #194

merged 6 commits into from
May 17, 2024

Conversation

johnstcn
Copy link
Member

@johnstcn johnstcn commented May 17, 2024

Part of #178

In order to update our branch of Kaniko, we need to first update to go1.22.
This is not currently possible while depending on codersdk.

  • Manually vendored relevant parts of codersdk and agentsdk into internal/notcodersdk
  • Replaced existing usage of codersdk / agentsdk with internal/notcodersdk
  • Added test for coder log sending functionality

Before:

$ du -sh ./scripts/envbuilder-amd64 
64M     ./scripts/envbuilder-amd64

After:

$ du -sh ./scripts/envbuilder-amd64 
45M     ./scripts/envbuilder-amd64

@johnstcn johnstcn self-assigned this May 17, 2024
Copy link
Contributor

@dannykopping dannykopping left a comment

Choose a reason for hiding this comment

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

LGTM

Do we anticipate we will be in this state for some time? If so, a README in the notcodersdk would be helpful for archaeological purposes. Optionally one step further would be to add a script to clone the repo and lint to ensure we aren't drifting too far from the original repo, but this may be overkill for now.

@johnstcn
Copy link
Member Author

Do we anticipate we will be in this state for some time?

#193 exists to investigate removing this direct dependency. Hopefully as a result of either that or another follow-up PR we can then remove this entirely.

If so, a README in the notcodersdk would be helpful for archaeological purposes.

This is a good idea either way 👍

Copy link
Member

@mafredri mafredri left a comment

Choose a reason for hiding this comment

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

notcodersdk

This sounds sneaky. 😂

cmd/envbuilder/main.go Outdated Show resolved Hide resolved
@johnstcn johnstcn merged commit 85290fa into main May 17, 2024
3 checks passed
@johnstcn johnstcn deleted the cj/notcodersdk branch May 17, 2024 11:58
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.

3 participants