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: add stop subcommand #1333

Merged
merged 5 commits into from
Jan 25, 2021
Merged

Conversation

tokers
Copy link
Contributor

@tokers tokers commented Jan 19, 2021

Please answer these questions before submitting a pull request

  • Why submit this pull request?

  • Bugfix

  • New feature provided

  • Improve performance

  • Backport patches

  • Related issues

close #741


Bugfix

  • Description

  • How to fix?


New feature or improvement

  • Describe the details and related test reports.

Please add the corresponding test cases if necessary.


Backport patches

  • Why need to backport?

  • Source branch

  • Related commits and pull requests

  • Target branch

@tokers tokers requested a review from nic-chen January 19, 2021 07:06
@tokers
Copy link
Contributor Author

tokers commented Jan 19, 2021

@starsz

@imjoey
Copy link
Member

imjoey commented Jan 19, 2021

@tokers , thank you for this. Just FYI, the file api/manager is in this PR. 😄

@nic-chen
Copy link
Member

@tokers , thank you for this. Just FYI, the file api/manager is in this PR. 😄

yes, we should not submit the binary

@tokers
Copy link
Contributor Author

tokers commented Jan 20, 2021

@tokers , thank you for this. Just FYI, the file api/manager is in this PR. 😄

Oh my gosh, i'll remove it, sorry~

@codecov-io
Copy link

codecov-io commented Jan 20, 2021

Codecov Report

Merging #1333 (35cb72e) into master (8852e63) will decrease coverage by 0.28%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1333      +/-   ##
==========================================
- Coverage   44.31%   44.02%   -0.29%     
==========================================
  Files          32       33       +1     
  Lines        1961     1976      +15     
==========================================
+ Hits          869      870       +1     
- Misses        981      995      +14     
  Partials      111      111              
Impacted Files Coverage Δ
api/internal/utils/pid.go 0.00% <0.00%> (ø)
api/internal/core/store/store.go 81.01% <0.00%> (+0.63%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8852e63...35cb72e. Read the comment docs.

api/cmd/managerapi.go Show resolved Hide resolved
@idbeta
Copy link
Contributor

idbeta commented Jan 20, 2021

You should add instructions in the document.

Copy link
Member

@membphis membphis left a comment

Choose a reason for hiding this comment

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

need to update the doc, help the user know this command

@juzhiyuan juzhiyuan added the documentation Improvements or additions to documentation label Jan 21, 2021
@membphis
Copy link
Member

ping @tokers

@tokers
Copy link
Contributor Author

tokers commented Jan 21, 2021

Ok, will add it as soon as Possible.

@tokers
Copy link
Contributor Author

tokers commented Jan 22, 2021

@membphis @idbeta Added.

@membphis membphis requested a review from nic-chen January 22, 2021 04:15
@membphis
Copy link
Member

@membphis @idbeta Added.

LGTM

@juzhiyuan
Copy link
Member

ping @starsz @imjoey

@idbeta
Copy link
Contributor

idbeta commented Jan 25, 2021

@membphis @idbeta Added.

LGTM

@juzhiyuan
Copy link
Member

ping @tokers

Copy link
Member

@imjoey imjoey left a comment

Choose a reason for hiding this comment

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

/lgtm

Thanks @tokers .

@starsz starsz merged commit 39aac5b into apache:master Jan 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implmement sub-command like stop, restart for manager-api
8 participants