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

Documentation (Code Review) #79

Open
jamiecoe opened this issue Oct 14, 2018 · 1 comment
Open

Documentation (Code Review) #79

jamiecoe opened this issue Oct 14, 2018 · 1 comment

Comments

@jamiecoe
Copy link

jamiecoe commented Oct 14, 2018

ReadMe

Great start! Nice to see you've already got a license / link to site / info on deployment & technical commands 👍

Some more things you could add:

  • A small description of what the project is
  • Command for running tests
  • Dependencies Eg: a config.env with certain values, Airtable
  • Build status icon from travis, to show your tests are passing for Prod version
  • Tech stack
  • Images of any design prototypes
  • What you did in this sprint
  • User testing results
  • What you would do in future sprints

This inFact one is nice example of a ReadMe that tells the story of project in lots of detail.

ReadMes are great for your portfolios - the repository will not only show off your code but the entire project process (design / user-testing / project managing / prototyping) to future employers or free-lance clients.

Issues

Great to see you labelling issues! It would be useful to label PRs as well (eg: ‘in progress’ if it’s still being worked on and ‘awaiting review’ when you want the team to review it). That way, the team know when a PR is ready.

Excellent use of ‘Projects’ board 🎉 Really nice to be able to see progress of Issues.

Milestones are handy for grouping issues together into Sprints. It makes it easy to track what you are working on in a current Sprint and the progress bar shows how far you’re off from finishing.

I see you’re referencing other issues here. You can link to other issues or PRs using a hashtag with the number of the issues/PR (eg: #53).

Commit messages

For commit messages, focus on why you've made the commit, not what is in the commit (the code itself should show this).

A good example of one of your commit messages that focuses on the why:

clean up database files as we are using airtables now 

This is great because it explains why you made the changes - which is easy for me (as an outside developer) or you in a few months/years time to understand why you made that change. Remember the why is the important information that can easily get lost, so documenting it with commit messages is really helpful.

Also, try to avoid the temptation to have multiple things in one commit. Eg:

implement displaying one-detailed-event on clicking div and implement airtables. 

What if you wanted to roll back on the one-detailed-event stuff, but keep airtable? If these were two separate commits, you could easily role back one and keep the other.

@MissArray
Copy link
Collaborator

Hi Jamie,

There's already a README. with the commands for running tests at the bottom, but of course, as you point out, the file is still incomplete. Working on it...

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

No branches or pull requests

2 participants