-
Notifications
You must be signed in to change notification settings - Fork 7
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
Review Issue #84 - Weight Tracker #92
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this is already a lot more clear/consistent than the original. I made some minor suggestions, mostly optional, that I think help even further in terms of clarity.
@Alumniminium I just noticed that this is actually a PR into main from initial-review. I thought we were doing them from other branches/our forks into initial-review, then just doing a big merge into main at the end of the project? I don't know what's better TBH, I just proposed to merge everything into initial review in the discussion I had posted and no one mentioned anything to the contrary, so I just assumed we were going with that. I can't commit to the repo anyway, so I am just doing from my fork into initial-review |
Ah yes, that would make a lot more sense. I was a bit confused what you had in mind. Will do that in the future. I'll just close this PR and not merge it. Thanks for your changes. I'll make sure to include your changes! One day my english will be up to standards :D |
Come on, it was really good and clean! I was just being a very picky editor :) |
Here's my updated weight-tracker.md to close #84 and close #14
I've fixed the grammar, markdown headings and rewrote it a bit.
Someone else might want to add a new screenshot?