lnd icon indicating copy to clipboard operation
lnd copied to clipboard

Modified the listchanneltxn subcommand

Open Chinwendu20 opened this issue 1 year ago • 19 comments

Change Description

start_height must be <= end_height, error otherwise transactions are returned in direct order (oldest first) an additional flag --reverse is added to change the order (--reverse true, recent first) Modified docs by modifying comments in the listchaintxn description command

Steps to Test

Carry out a debug installation using this command: make && make install

Test command: ./lncli-debug --no-macaroons listchaintxns --start_height 200 --end_height 7000| grep block_height Expected outcome: transactions are returned in the order, oldest first image

./lncli-debug --no-macaroons listchaintxns --start_height 7000 --end_height 200| grep block_height Expected outcome: Error image

./lncli-debug --no-macaroons listchaintxns --start_height 200 --end_height 7000 --reverse| grep block_height Expected outcome: transactions are returned in the reverse order, newest first image

Change after review

Test command: ./lncli-debug --no-macaroons listchaintxns --start_height 200 --end_height 7000 --reverse| grep block_height Expected outcome: transactions are returned in the order, oldest first image

./lncli-debug --no-macaroons listchaintxns --start_height 7000 --end_height 200| grep block_height Expected outcome: Error image

./lncli-debug --no-macaroons listchaintxns --start_height 200 --end_height 7000| grep block_height Expected outcome: transactions are returned in the order, newest first image

Chinwendu20 avatar Mar 08 '23 22:03 Chinwendu20

Thanks would this involve amending existing commit messages or would I have to take Git HEAD to the tip before I started making changes then make all the changes one after the other, to get the commit to look like that

Chinwendu20 avatar Mar 14 '23 05:03 Chinwendu20

Thanks would this involve amending existing commit messages or would I have to take Git HEAD to the tip before I started making changes then make all the changes one after the other, to get the commit to look like that

In this case I would take the second approach. You can reset your head while keeping the changes (git reset --soft HEAD~2), then add the couple of fixes + create the right commit structure (git commit -p is your friend here).

When the log is in good shape remember to add one last commit updating the release notes (you can check other PRs to see how it's done)

positiveblue avatar Mar 14 '23 05:03 positiveblue

ok working on it @positiveblue

Chinwendu20 avatar Mar 27 '23 19:03 Chinwendu20

I have made the change @positiveblue

Chinwendu20 avatar Mar 28 '23 08:03 Chinwendu20

@Chinwendu20 - are you planning to continue work on this? if so, please click the "re-request" review button :)

ellemouton avatar Apr 18 '23 11:04 ellemouton

Yes, thank you. Just seeing that there is a conflict. I will fix it and re-request a review @ellemouton

Chinwendu20 avatar Apr 18 '23 15:04 Chinwendu20

@Chinwendu20 - it seems that not all the comments left in the previous review have been addressed?

ellemouton avatar Apr 20 '23 14:04 ellemouton

Thanks please do you know how I can reply to those reviews. Last time I did that it showed pending and @positiveblue could not see it. image

Chinwendu20 avatar Apr 21 '23 09:04 Chinwendu20

you have to go into "Files Changed" and then click the "Review Changes" button. Then you will be able to click "Submitt Review" which would submit your comments

Screenshot 2023-04-21 at 11 06 57

ellemouton avatar Apr 21 '23 09:04 ellemouton

Okay thank you

Chinwendu20 avatar Apr 21 '23 09:04 Chinwendu20

Hello @positiveblue please I have made changes to this PR do you mind reviewing it again?

Chinwendu20 avatar Sep 09 '23 19:09 Chinwendu20

Reverting the lnrpc changes... hold on

Chinwendu20 avatar Sep 09 '23 19:09 Chinwendu20

All done. Ready for review.

Chinwendu20 avatar Sep 09 '23 20:09 Chinwendu20

Hello, I have added the release note and fixed the lint error, do you mind rerunning the workflow?

Chinwendu20 avatar Sep 18 '23 09:09 Chinwendu20

So sorry for the back and forth, for some reason. I cannot seem to see any linting error like it is on the CI when I run make lint locally: image

Chinwendu20 avatar Sep 18 '23 10:09 Chinwendu20

@Chinwendu20 Is there anything I could help with so that we could get this PR? The CI regards Linting the code seems passed.

mohamedawnallah avatar Mar 06 '24 19:03 mohamedawnallah

@Chinwendu20 I had a similar issue regards running the make lint command since I've 8GB of RAM. This is solved by passing the number of concurrent workers to be 1 so the full command make lint workers=1 hope it works for you :)

So sorry for the back and forth, for some reason. I cannot seem to see any linting error like it is on the CI when I run make lint locally:

mohamedawnallah avatar Mar 10 '24 20:03 mohamedawnallah

Thanks @mohamedawnallah. I think I was waiting for review here?

Chinwendu20 avatar Mar 11 '24 11:03 Chinwendu20

@positiveblue: review reminder

lightninglabs-deploy avatar May 20 '24 17:05 lightninglabs-deploy