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

Ntate/appdev/rebased #1237

Merged
merged 46 commits into from
Sep 22, 2022
Merged

Conversation

nicktate
Copy link
Contributor

@nicktate nicktate commented Sep 20, 2022

Descriptions

What does this pull request accomplish

  • Adds in the doctl app dev command subtree
  • Adds command/charm package with foundational constructs to enable interactivity throughout commands

Additional Notes

This PR is ready to review for merge-ability into main with the following caveats:

The change set is massive 😅 but a large majority is related to vendor changes; this code went through an initial set of reviews to feature/appdev

@nicktate nicktate force-pushed the ntate/appdev/rebased branch 3 times, most recently from dbe0f8c to 568efdc Compare September 20, 2022 16:07
@nicktate
Copy link
Contributor Author

Error: commands/displayers/reserved_ip.go:58:19: f.ProjectID undefined (type do.ReservedIP has no field or method ProjectID)

The current test failure is due to the godo replacement; it will be resolved once we have updated this PR w/ the relevant godo release

@nicktate nicktate force-pushed the ntate/appdev/rebased branch 2 times, most recently from 6506581 to a0c801e Compare September 20, 2022 18:27
nicktate and others added 23 commits September 21, 2022 13:19
* apps: add base scaffolding for app dev command subtree

* charm templates: change up method signatures for easier use

* only use 1.18.x in ci

* add internal package to unit test

* disable interactive mode

* only use logging pager in interactive mode

Co-authored-by: Kamal Nasser <kamal@digitalocean.com>
…n#1214)

* Add snap support for docker interface for app dev builds

* add snap warning message
* add support for local cnb caching

* address pr feedback
@nicktate nicktate force-pushed the ntate/appdev/rebased branch from 94194a2 to e8dfe8f Compare September 21, 2022 20:43
Copy link
Member

@andrewsomething andrewsomething left a comment

Choose a reason for hiding this comment

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

Over all this looks great. I've given it a pretty thorough review, but admittedly need to rely on you all for some of the finer details of the build process. A few comments inline, but they are all pretty minor.

commands/apps_dev.go Outdated Show resolved Hide resolved
commands/apps_dev_config.go Outdated Show resolved Hide resolved
commands/apps_dev.go Show resolved Hide resolved
internal/apps/builder/builder.go Show resolved Hide resolved
commands/apps_dev.go Show resolved Hide resolved
commands/apps_dev_config.go Outdated Show resolved Hide resolved
@andrewsomething andrewsomething self-requested a review September 22, 2022 18:19
Copy link
Member

@andrewsomething andrewsomething left a comment

Choose a reason for hiding this comment

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

🚀

@andrewsomething andrewsomething merged commit de1918a into digitalocean:main Sep 22, 2022
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