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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove stack commands #4352

Merged
merged 9 commits into from
Jun 25, 2024
Merged

Conversation

jLopezbarb
Copy link
Contributor

@jLopezbarb jLopezbarb commented Jun 18, 2024

Proposed changes

Fixes DEV-427

  • Remove command "okteto stack" and its subcommands (deploy, destroy, endpoints)
  • Remove dead code if any
  • Remove unit and integration tests
  • "okteto deploy stack" GitHub Action is removed from the Marketplace
  • okteto/deploy-stack action repository is flagged as deprecated & archived add deprecation message聽deploy-stack#12
  • "okteto destroy stack" GitHub Action is removed from the Marketplace
  • okteto/destroy-stack action repository is flagged as deprecated & archived add deprecation message聽destroy-stack#11

How to validate

  1. Using movies-with-compose
  2. Run okteto deploy and check that its deployed correctly
  3. Run okteto endpoints and check that it works
  4. Run okteto destroy and check that everything is destroyed correctly

CLI Quality Reminders 馃敡

For both authors and reviewers:

  • Scrutinize for potential regressions
  • Ensure key automated tests are in place
  • Build the CLI and test using the validation steps
  • Assess Developer Experience impact (log messages, performances, etc)
  • If too broad, consider breaking into smaller PRs
  • Adhere to our code style and code review guidelines

jLopezbarb and others added 5 commits June 14, 2024 17:51
* refactor: remove e2e tests code about push

Signed-off-by: Javier Lopez <[email protected]>

* refactor: remove autodiscovery for push

Signed-off-by: Javier Lopez <[email protected]>

* refactor: remove code related to okteto push

Signed-off-by: Javier Lopez <[email protected]>

* integration: remove push from ci

Signed-off-by: Javier Lopez <[email protected]>

* refactor: remove unused code

Signed-off-by: Javier Lopez <[email protected]>

---------

Signed-off-by: Javier Lopez <[email protected]>
* remove login cmd

Signed-off-by: Andrea Falzetti <[email protected]>

* remove gha

Signed-off-by: Andrea Falzetti <[email protected]>

---------

Signed-off-by: Andrea Falzetti <[email protected]>
Signed-off-by: Javier Lopez <[email protected]>
@jLopezbarb jLopezbarb requested a review from a team as a code owner June 18, 2024 12:06
Copy link

codecov bot commented Jun 18, 2024

Codecov Report

Attention: Patch coverage is 0% with 25 lines in your changes missing coverage. Please review.

Project coverage is 45.76%. Comparing base (665b48d) to head (30b58e4).

Additional details and impacted files
@@                        Coverage Diff                        @@
##           ultra-mega-release-branch-3.0    #4352      +/-   ##
=================================================================
+ Coverage                          45.25%   45.76%   +0.50%     
=================================================================
  Files                                361      354       -7     
  Lines                              28646    28326     -320     
=================================================================
- Hits                               12963    12962       -1     
+ Misses                             14588    14272     -316     
+ Partials                            1095     1092       -3     

@andreafalzetti
Copy link
Contributor

@jLopezbarb I think you selected master by mistake as base branch

@jLopezbarb jLopezbarb changed the base branch from master to ultra-mega-release-branch-3.0 June 18, 2024 12:45
@jLopezbarb jLopezbarb marked this pull request as draft June 18, 2024 12:46
@jLopezbarb jLopezbarb marked this pull request as ready for review June 18, 2024 14:19
Signed-off-by: Javier Lopez <[email protected]>
Copy link
Contributor

@andreafalzetti andreafalzetti left a comment

Choose a reason for hiding this comment

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

Approving to unlock the PR but please check the conflicts and comment 馃檹

@@ -185,8 +184,6 @@ func main() {
root.AddCommand(remoterun.RemoteRun(ctx, k8sLogger))
root.AddCommand(test.Test(ctx, ioController, k8sLogger, at))

// deprecated
root.AddCommand(stack.Stack(ctx, at, insights, ioController))
root.AddCommand(pipeline.Pipeline(ctx))
Copy link
Member

Choose a reason for hiding this comment

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

deprecated is still applying for pipeline?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, I removed the //deprecated to reflect it. We should probably needed to put it on the side instead of the top of the function.

@@ -85,8 +85,30 @@ const (
maxRestartsToConsiderFailed = 3
)

func (sd *Stack) RunDeploy(ctx context.Context, s *model.Stack, options *DeployOptions) error {
Copy link
Member

Choose a reason for hiding this comment

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

if stack is gone, can we rename this to Compose or similar?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We removed the command but not the syntax so I think it's ok to leave the name like this

Copy link
Member

@teresaromero teresaromero left a comment

Choose a reason for hiding this comment

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

minor comments

@jLopezbarb jLopezbarb merged commit f7f0f3e into ultra-mega-release-branch-3.0 Jun 25, 2024
10 of 12 checks passed
@jLopezbarb jLopezbarb deleted the jlo/remove-stack branch June 25, 2024 14:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants