Skip to content

Conversation

chong-he
Copy link
Member

Issue Addressed

Update Information in Lighthouse Book

Proposed Changes

  • move Validator Graffiti from Advanced Usage to Validator Management
  • update API response and command
  • some items that aren't too sure I put it in comment, which can be seen in raw/review format but not live

Additional Info

Please provide any additional information. For example, future considerations
or information useful for reviewers.

@paulhauner paulhauner added the ready-for-review The code is ready for review label May 22, 2023
@paulhauner paulhauner self-assigned this May 22, 2023
Copy link
Member

@paulhauner paulhauner left a comment

Choose a reason for hiding this comment

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

Wow, this is a big effort! It makes lots of overdue fixes and I think it's a great improvement! 🚀

I've made a bunch of small suggestions. FYI, if you use the "Conversation" tab then you can only implement suggestions one-by-one, but if you use the "Files changed" tab you can batch them (this is faster, IMO).

I would also like to request changing all instances of sudo cat to just cat. I can see how the sudo is useful, however we tend to leave sudo out of examples since it can be dangerous. I don't think your use is dangerous, but I'd prefer to avoid it if possible. Perhaps adding a note that sudo maybe required is nice?

@paulhauner paulhauner added waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed ready-for-review The code is ready for review labels May 29, 2023
@chong-he
Copy link
Member Author

Wow, this is a big effort! It makes lots of overdue fixes and I think it's a great improvement! 🚀

I've made a bunch of small suggestions. FYI, if you use the "Conversation" tab then you can only implement suggestions one-by-one, but if you use the "Files changed" tab you can batch them (this is faster, IMO).

I would also like to request changing all instances of sudo cat to just cat. I can see how the sudo is useful, however we tend to leave sudo out of examples since it can be dangerous. I don't think your use is dangerous, but I'd prefer to avoid it if possible. Perhaps adding a note that sudo maybe required is nice?

Thanks so much for the review. I have committed all suggestions, removed comments and sudo.

@chong-he chong-he requested a review from paulhauner May 31, 2023 01:25
Copy link
Member

@paulhauner paulhauner left a comment

Choose a reason for hiding this comment

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

Looks great, thank you!

@paulhauner paulhauner added ready-for-merge This PR is ready to merge. and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels Jun 1, 2023
@paulhauner
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Jun 1, 2023
## Issue Addressed

Update Information in Lighthouse Book

## Proposed Changes

- move Validator Graffiti from Advanced Usage to Validator Management
- update API response and command
- some items that aren't too sure I put it in comment, which can be seen in raw/review format but not live


## Additional Info

Please provide any additional information. For example, future considerations
or information useful for reviewers.


Co-authored-by: chonghe <[email protected]>
@bors
Copy link

bors bot commented Jun 1, 2023

Build failed (retrying...):

bors bot pushed a commit that referenced this pull request Jun 1, 2023
## Issue Addressed

Update Information in Lighthouse Book

## Proposed Changes

- move Validator Graffiti from Advanced Usage to Validator Management
- update API response and command
- some items that aren't too sure I put it in comment, which can be seen in raw/review format but not live


## Additional Info

Please provide any additional information. For example, future considerations
or information useful for reviewers.


Co-authored-by: chonghe <[email protected]>
bors bot pushed a commit that referenced this pull request Jun 1, 2023
## Issue Addressed

Update Information in Lighthouse Book

## Proposed Changes

- move Validator Graffiti from Advanced Usage to Validator Management
- update API response and command
- some items that aren't too sure I put it in comment, which can be seen in raw/review format but not live


## Additional Info

Please provide any additional information. For example, future considerations
or information useful for reviewers.


Co-authored-by: chonghe <[email protected]>
@bors
Copy link

bors bot commented Jun 1, 2023

Build failed (retrying...):

bors bot pushed a commit that referenced this pull request Jun 2, 2023
## Issue Addressed

Update Information in Lighthouse Book

## Proposed Changes

- move Validator Graffiti from Advanced Usage to Validator Management
- update API response and command
- some items that aren't too sure I put it in comment, which can be seen in raw/review format but not live


## Additional Info

Please provide any additional information. For example, future considerations
or information useful for reviewers.


Co-authored-by: chonghe <[email protected]>
@bors
Copy link

bors bot commented Jun 2, 2023

Build failed:

@paulhauner
Copy link
Member

bors r-

@paulhauner
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Jun 2, 2023
## Issue Addressed

Update Information in Lighthouse Book

## Proposed Changes

- move Validator Graffiti from Advanced Usage to Validator Management
- update API response and command
- some items that aren't too sure I put it in comment, which can be seen in raw/review format but not live


## Additional Info

Please provide any additional information. For example, future considerations
or information useful for reviewers.


Co-authored-by: chonghe <[email protected]>
@bors
Copy link

bors bot commented Jun 2, 2023

Build failed (retrying...):

bors bot pushed a commit that referenced this pull request Jun 2, 2023
## Issue Addressed

Update Information in Lighthouse Book

## Proposed Changes

- move Validator Graffiti from Advanced Usage to Validator Management
- update API response and command
- some items that aren't too sure I put it in comment, which can be seen in raw/review format but not live


## Additional Info

Please provide any additional information. For example, future considerations
or information useful for reviewers.


Co-authored-by: chonghe <[email protected]>
@bors
Copy link

bors bot commented Jun 2, 2023

Build failed (retrying...):

bors bot pushed a commit that referenced this pull request Jun 2, 2023
## Issue Addressed

Update Information in Lighthouse Book

## Proposed Changes

- move Validator Graffiti from Advanced Usage to Validator Management
- update API response and command
- some items that aren't too sure I put it in comment, which can be seen in raw/review format but not live


## Additional Info

Please provide any additional information. For example, future considerations
or information useful for reviewers.


Co-authored-by: chonghe <[email protected]>
@bors
Copy link

bors bot commented Jun 2, 2023

Build failed (retrying...):

bors bot pushed a commit that referenced this pull request Jun 2, 2023
## Issue Addressed

Update Information in Lighthouse Book

## Proposed Changes

- move Validator Graffiti from Advanced Usage to Validator Management
- update API response and command
- some items that aren't too sure I put it in comment, which can be seen in raw/review format but not live


## Additional Info

Please provide any additional information. For example, future considerations
or information useful for reviewers.


Co-authored-by: chonghe <[email protected]>
@bors
Copy link

bors bot commented Jun 2, 2023

Build failed:

@paulhauner
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Jun 2, 2023
## Issue Addressed

Update Information in Lighthouse Book

## Proposed Changes

- move Validator Graffiti from Advanced Usage to Validator Management
- update API response and command
- some items that aren't too sure I put it in comment, which can be seen in raw/review format but not live


## Additional Info

Please provide any additional information. For example, future considerations
or information useful for reviewers.


Co-authored-by: chonghe <[email protected]>
@bors
Copy link

bors bot commented Jun 2, 2023

Build failed (retrying...):

bors bot pushed a commit that referenced this pull request Jun 2, 2023
## Issue Addressed

Update Information in Lighthouse Book

## Proposed Changes

- move Validator Graffiti from Advanced Usage to Validator Management
- update API response and command
- some items that aren't too sure I put it in comment, which can be seen in raw/review format but not live


## Additional Info

Please provide any additional information. For example, future considerations
or information useful for reviewers.


Co-authored-by: chonghe <[email protected]>
@bors
Copy link

bors bot commented Jun 2, 2023

Build failed (retrying...):

@michaelsproul
Copy link
Member

bors r-
bors r+

@bors
Copy link

bors bot commented Jun 2, 2023

Canceled.

bors bot pushed a commit that referenced this pull request Jun 2, 2023
## Issue Addressed

Update Information in Lighthouse Book

## Proposed Changes

- move Validator Graffiti from Advanced Usage to Validator Management
- update API response and command
- some items that aren't too sure I put it in comment, which can be seen in raw/review format but not live


## Additional Info

Please provide any additional information. For example, future considerations
or information useful for reviewers.


Co-authored-by: chonghe <[email protected]>
@bors
Copy link

bors bot commented Jun 2, 2023

Pull request successfully merged into unstable.

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot changed the title Update Lighthouse Book API and Advanced Usage section [Merged by Bors] - Update Lighthouse Book API and Advanced Usage section Jun 2, 2023
@bors bors bot closed this Jun 2, 2023
divagant-martian pushed a commit to divagant-martian/lighthouse that referenced this pull request Jun 7, 2023
## Issue Addressed

Update Information in Lighthouse Book

## Proposed Changes

- move Validator Graffiti from Advanced Usage to Validator Management
- update API response and command
- some items that aren't too sure I put it in comment, which can be seen in raw/review format but not live


## Additional Info

Please provide any additional information. For example, future considerations
or information useful for reviewers.


Co-authored-by: chonghe <[email protected]>
ghost pushed a commit to oone-world/lighthouse that referenced this pull request Jul 13, 2023
## Issue Addressed

Update Information in Lighthouse Book

## Proposed Changes

- move Validator Graffiti from Advanced Usage to Validator Management
- update API response and command
- some items that aren't too sure I put it in comment, which can be seen in raw/review format but not live


## Additional Info

Please provide any additional information. For example, future considerations
or information useful for reviewers.


Co-authored-by: chonghe <[email protected]>
@chong-he chong-he deleted the book-api branch December 3, 2023 09:39
Woodpile37 pushed a commit to Woodpile37/lighthouse that referenced this pull request Jan 6, 2024
## Issue Addressed

Update Information in Lighthouse Book

## Proposed Changes

- move Validator Graffiti from Advanced Usage to Validator Management
- update API response and command
- some items that aren't too sure I put it in comment, which can be seen in raw/review format but not live


## Additional Info

Please provide any additional information. For example, future considerations
or information useful for reviewers.


Co-authored-by: chonghe <[email protected]>
Woodpile37 pushed a commit to Woodpile37/lighthouse that referenced this pull request Jan 6, 2024
## Issue Addressed

Update Information in Lighthouse Book

## Proposed Changes

- move Validator Graffiti from Advanced Usage to Validator Management
- update API response and command
- some items that aren't too sure I put it in comment, which can be seen in raw/review format but not live


## Additional Info

Please provide any additional information. For example, future considerations
or information useful for reviewers.


Co-authored-by: chonghe <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation ready-for-merge This PR is ready to merge.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants