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

Clean up README #239

Merged

Conversation

chrisguida
Copy link
Collaborator

Clean up README and add warning about Lampo forgetting the seed on restart.
Also fix some typos in log messages.

Add warning about Lampo forgetting the seed on restart
Also fix some typos in log messages
@chrisguida chrisguida force-pushed the cguida/cleanup-readme branch from 539d6c8 to 2cf4448 Compare May 29, 2024 19:21
Copy link
Owner

@vincenzopalazzo vincenzopalazzo left a comment

Choose a reason for hiding this comment

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

Thanks!

ACK 2cf4448

Next time, could you add the changes in separate commits? For example:

  1. Change the README
  2. Fix the log typos

This way, if we want to create an LTS version like the kernel, we can cherry-pick the commits easily into another branch without causing a mess of changes. 😸

However, thanks for your typo fixes!

@vincenzopalazzo vincenzopalazzo merged commit a973f15 into vincenzopalazzo:main May 30, 2024
3 of 5 checks passed
@chrisguida
Copy link
Collaborator Author

chrisguida commented May 30, 2024

Sure! The reason I did it this way is because, since the typos in the log messages were referenced in the README, it seemed weird to fix them in the README, then fix them in the places the README is referencing, in different commits.

@vincenzopalazzo
Copy link
Owner

Ah make sense! but it is better keep separate also if related, in this way we can cherry-pick just the code that we need

@chrisguida
Copy link
Collaborator Author

Ok I'll keep that in mind for future contributions, thanks for providing feedback :)

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.

2 participants