Skip to content

Conversation

@mtx2d
Copy link
Contributor

@mtx2d mtx2d commented Jul 15, 2020

Fix #1176

Pre-submission checklist:

Please answer each of these after submitting your pull request:

  • Did you explain what problem does this PR solve? Or what new features have been added?
    change: remove the plugin heartbeat #1776 Says that heartbeat is no longer needed, we can remove this.
  • Have you added corresponding test cases?
    Checked that there is not test cases related to heartbeat.
    Removing this file does not break existing test suit.
  • Have you modified the corresponding document?
    Link to document including heartbeart.
    There are three instances, two of them in the change log, and last one discussing in general service discovery.
    No need to modify any one of them.
  • Is this PR backward compatible?
    Not sure how to check this.

@membphis
Copy link
Member

@mtx2d
Copy link
Contributor Author

mtx2d commented Jul 15, 2020

@mtx2d please take a look at the output of CI, eg: https://github.com/apache/incubator-apisix/pull/1845/checks?check_run_id=871655239#step:6:272

Cannot run the test locally, so running this with CI.
In the meantime, could you please say more on why we no longer need heartbeat?
I can read the test failure and all, but design wise understanding the motivation will help me to create the correct PR.

@mtx2d
Copy link
Contributor Author

mtx2d commented Jul 15, 2020

Trying to figure out how to trigger CI.

@mtx2d
Copy link
Contributor Author

mtx2d commented Jul 15, 2020

@moonming @membphis , is it possible to force trigger a CI?
I made new commit to my local repo mtx2d/incubato-apisix, but the CI still looks at the old states.(my master should be 11 commits ahead but the CI still believes I am only 5 commits ahead).

How can I force trigger a CI?

@mtx2d
Copy link
Contributor Author

mtx2d commented Jul 15, 2020

@moonming @membphis , is it possible to force trigger a CI?
I made new commit to my local repo mtx2d/incubato-apisix, but the CI still looks at the old states.(my master should be 11 commits ahead but the CI still believes I am only 5 commits ahead).

How can I force trigger a CI?

Seems like the CI picks it up now.

@mtx2d
Copy link
Contributor Author

mtx2d commented Jul 15, 2020

The CI fails due to CI server network issue: https://github.com/apache/incubator-apisix/pull/1845/checks?check_run_id=874200614#step:3:68

Will make a small change and trigger another CI run.

@mtx2d
Copy link
Contributor Author

mtx2d commented Jul 15, 2020

According to this doc, we can close and reopen, which will trigger a new CI run without making changes.

@mtx2d mtx2d closed this Jul 15, 2020
@mtx2d mtx2d reopened this Jul 15, 2020
@mtx2d
Copy link
Contributor Author

mtx2d commented Jul 16, 2020

Hi @membphis,
Do we think this is PR good to move forward :D?

Copy link
Member

@moonming moonming left a comment

Choose a reason for hiding this comment

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

LGTM

@mtx2d mtx2d requested a review from membphis July 16, 2020 05:47
@membphis membphis merged commit 24d7007 into apache:master Jul 16, 2020
@membphis
Copy link
Member

@mtx2d many thx, merged already

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