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.

Sign up for GitHub

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

Jump to bottom

Adding chia peer command #10760

Closed
wants to merge 33 commits into from
Closed

Adding chia peer command #10760

wants to merge 33 commits into from

Conversation

wallentx
Copy link
Contributor

@wallentx wallentx commented Mar 16, 2022 •
edited by jack60612
Loading

Addition of chia peer command for (wallet, farmer, full node and harvester) & slight refactor of chia show.

@wallentx wallentx requested a review from cmmarslender March 16, 2022 01:17
@lgtm-com
Copy link

lgtm-com bot commented Mar 16, 2022

This pull request introduces 4 alerts and fixes 1 when merging f3ba37f into 185a483 - view on LGTM.com

new alerts:

  • 3 for Unused import
  • 1 for Clear-text logging of sensitive information

fixed alerts:

  • 1 for Clear-text logging of sensitive information

@lgtm-com
Copy link

lgtm-com bot commented Mar 21, 2022

This pull request introduces 1 alert and fixes 1 when merging b8f68a5 into 9db85f3 - view on LGTM.com

new alerts:

  • 1 for Clear-text logging of sensitive information

fixed alerts:

  • 1 for Clear-text logging of sensitive information

altendky
altendky reviewed Mar 21, 2022
View reviewed changes
wallentx and others added 3 commits March 22, 2022 15:46
@wallentx @altendky
Update chia/cmds/peer.py
87cb68e 
Adding as suggested

Co-authored-by: Kyle Altendorf <[email protected]>
@wallentx
Making suggested changes to peer.py
84e6970
@wallentx
Merge branch 'wallentx/chia-peer-command' of github.com:Chia-Network/…
98adcae 
…chia-blockchain into wallentx/chia-peer-command
@lgtm-com
Copy link

lgtm-com bot commented Mar 22, 2022

This pull request introduces 1 alert and fixes 1 when merging 98adcae into b7a1c9c - view on LGTM.com

new alerts:

  • 1 for Clear-text logging of sensitive information

fixed alerts:

  • 1 for Clear-text logging of sensitive information

@github-actions
Copy link
Contributor

github-actions bot commented May 7, 2022

This PR has been flagged as stale due to no activity for over 60 days. It will not be automatically closed, but it has been given a stale-pr label and should be manually reviewed by the relevant parties.

@github-actions github-actions bot added the stale-pr Flagged as stale and in need of manual review label May 7, 2022
@github-actions
Copy link
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot added the merge_conflict Branch has conflicts that prevent merge to main label May 24, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Jul 7, 2022

Conflicts have been resolved. A maintainer will review the pull request shortly.

@github-actions github-actions bot removed the merge_conflict Branch has conflicts that prevent merge to main label Jul 7, 2022
@jack60612 jack60612 removed the stale-pr Flagged as stale and in need of manual review label Jul 7, 2022
@jack60612 jack60612 requested review from mariano54 and jack60612 July 7, 2022 04:35
@lgtm-com
Copy link

lgtm-com bot commented Jul 7, 2022

This pull request introduces 1 alert when merging ab5a3aa into 6923ad8 - view on LGTM.com

new alerts:

  • 1 for Clear-text logging of sensitive information

@jack60612 @wallentx
finalize pr
df333b1 
very nice
i basically did half the work when i refactored it

Co-Authored-By: William Allen <[email protected]>
@jack60612 jack60612 requested a review from altendky July 7, 2022 04:51
@jack60612
Copy link
Contributor

Testing locally now

@jack60612
Copy link
Contributor

Local CLI tests successful!!

@jack60612 jack60612 marked this pull request as ready for review July 7, 2022 04:54
@jack60612 jack60612 removed their request for review July 7, 2022 04:55
@lgtm-com
Copy link

lgtm-com bot commented Jul 22, 2022

This pull request introduces 40 alerts when merging e8dfcd9 into edd60a1 - view on LGTM.com

new alerts:

  • 40 for Module-level cyclic import

mariano54
mariano54 previously approved these changes Jul 25, 2022
View reviewed changes
Copy link
Contributor

@mariano54 mariano54 left a comment

Choose a reason for hiding this comment

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

LGTM apart from small nit for naming

@mariano54 mariano54 added the ready_to_merge Submitter and reviewers think this is ready label Jul 25, 2022
@jack60612
rename function
0425770 
makes sense to me
@jack60612 jack60612 requested a review from mariano54 July 25, 2022 05:58
mariano54
mariano54 previously approved these changes Jul 25, 2022
View reviewed changes
@lgtm-com
Copy link

lgtm-com bot commented Jul 25, 2022

This pull request introduces 40 alerts when merging 0425770 into 3e996f8 - view on LGTM.com

new alerts:

  • 40 for Module-level cyclic import

@wallentx wallentx requested a review from a team July 25, 2022 16:46
@github-actions
Copy link
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot added the merge_conflict Branch has conflicts that prevent merge to main label Jul 25, 2022
@jack60612 jack60612 requested a review from mariano54 July 25, 2022 19:23
@github-actions github-actions bot removed the merge_conflict Branch has conflicts that prevent merge to main label Jul 25, 2022
@github-actions
Copy link
Contributor

Conflicts have been resolved. A maintainer will review the pull request shortly.

1 similar comment
@github-actions
Copy link
Contributor

Conflicts have been resolved. A maintainer will review the pull request shortly.

@lgtm-com
Copy link

lgtm-com bot commented Jul 25, 2022

This pull request introduces 40 alerts when merging 08732ee into 0bc54ae - view on LGTM.com

new alerts:

  • 40 for Module-level cyclic import

@jack60612 jack60612 requested a review from emlowe July 25, 2022 20:00
mariano54
mariano54 suggested changes Jul 26, 2022
View reviewed changes
Copy link
Contributor

@mariano54 mariano54 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 fix the LGTM issues (cyclic import)

@jack60612
Copy link
Contributor

Need to fix the LGTM issues (cyclic import)

they are wrong, i dont know what they are yapping about.

@jack60612 jack60612 requested a review from mariano54 July 26, 2022 03:08
@emlowe
Copy link
Contributor

emlowe commented Jul 26, 2022

Yea, LGTM has some issues - Kyle would actually like us to stop using it as it's not adding much value

@mariano54
Copy link
Contributor

mariano54 commented Jul 27, 2022 •
edited
Loading

We can't merge this PR if it adds 40 LGTM issues. There is a circular dependency added by this change.

Or should we just start ignoring it completely?

@jack60612
Copy link
Contributor

We can't merge this PR if it adds 40 LGTM issues. There is a circular dependency added by this change.

Or should we just start ignoring it completely?

i can assure you there is not a circular dependency, look at what LGTM says, if that was true then all the tests would fail.

@github-actions
Copy link
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot added the merge_conflict Branch has conflicts that prevent merge to main label Jul 28, 2022
@jack60612
Copy link
Contributor

closing this pr in favor of #12532

@jack60612 jack60612 closed this Jul 28, 2022
@jack60612 jack60612 deleted the wallentx/chia-peer-command branch July 28, 2022 19:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@altendky altendky altendky left review comments

@jack60612 jack60612 jack60612 left review comments

@emlowe emlowe Awaiting requested review from emlowe

@mariano54 mariano54 Awaiting requested review from mariano54

Assignees

@wallentx wallentx

@jack60612 jack60612

Labels
merge_conflict Branch has conflicts that prevent merge to main ready_to_merge Submitter and reviewers think this is ready
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

CLI naming
5 participants
@wallentx @jack60612 @emlowe @mariano54 @altendky