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

Simplification, Clarification, Robotification #118

Merged
merged 24 commits into from
Jun 5, 2018
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
02b6c37
Separate style guide from best practices and shorten up requirements
designatednerd Apr 24, 2018
217c552
Move project structure into best practices instead of a separate doc
designatednerd Apr 25, 2018
a557e86
clean up formatting on git and github doc, minor condtent adjustments
designatednerd Apr 25, 2018
125fd4b
clean up reusable components, remove outdated references
designatednerd Apr 25, 2018
e285ebd
clean up the general communication guide
designatednerd Apr 25, 2018
b273b92
Quick update of the releases page
designatednerd Apr 25, 2018
9c429ee
Cleanup hiring and combine repetitive bits.
designatednerd Apr 25, 2018
50e209d
move stuff in releases to git + github
designatednerd Apr 25, 2018
012f440
archive topic suggestions and weekly meetings,
designatednerd Apr 25, 2018
e0158ee
Add default swiftlint file
designatednerd Apr 25, 2018
8156375
Update readme.
designatednerd Apr 25, 2018
35a05a8
fix grammars
designatednerd Apr 25, 2018
8faab8a
make sure title capitalization is consistent + everything has an accu…
designatednerd Apr 26, 2018
01881ae
Merge branch 'master' into robots + fix merge conflicts
designatednerd May 2, 2018
a874c5d
spel mor gud
designatednerd May 2, 2018
f93bc7f
goods tool != good tools
designatednerd May 3, 2018
bb7e666
missing `a`
designatednerd May 3, 2018
5824d7c
rm excess word
designatednerd May 3, 2018
1bd0059
clarify why we're using certain casing conventions
designatednerd May 3, 2018
e3a6e29
Clarify reasoning for private unit test rule
designatednerd May 3, 2018
b1c334f
last remaining en_GB -> en_US
designatednerd May 3, 2018
94e308b
add a couple of exclusions to rules after trying to implement this on…
designatednerd May 3, 2018
769e177
kill off requirement about commit messages
designatednerd May 4, 2018
475823f
Better explanations of what rules we're partially disabling and why
designatednerd May 7, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
855 changes: 58 additions & 797 deletions BEST_PRACTICES.md

Large diffs are not rendered by default.

53 changes: 32 additions & 21 deletions COMMUNICATION.md
Original file line number Diff line number Diff line change
@@ -1,51 +1,62 @@
# Communicating within the iOS team
# Communicating with your colleaugues
Copy link
Contributor

Choose a reason for hiding this comment

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

One "u" too many. ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤦‍♀️


## Why we have guidelines about communicating
Within our iOS team we mostly communicate with each other via the magic of the internet, and that is great! It's easy and very efficient. But online communication can easily result into misunderstandings, which tend to be long, time consuming and inefficient. That's why we created some guidelines for how we like to communicate professionally, keep them in the back of your head when commenting on GitHub or Slack.
Within our iOS team, we mostly communicate with each other via the magic of the internet, and that's great! It's easy and very efficient, especially since we're often in different cities and/or countries.

## When to pay attention to the guidelines
Obviously we're communicating all the time. We can't read this document every time we're having something come out of our mouths or keyboards. That would result in silencing the world 🤐, and thats the opposite of the intention of these guidelines.
But online communication can easily result into misunderstandings, which tend to be long, time consuming and inefficient.These guidelines are meant to solve issues in our professional communication, like pull request comments, code review and more thorough discussions.

- **Only in professional communication**
When making jokes, talking about your favourite tea, or planning a team members birthday party surely a lot of things can go wrong in communication as well. But these guidelines are meant to solve issues in our professional communication, like pull request comments, code review and more thorough discussions.
Keep them in the back of your head when commenting on GitHub or Slack.

- **When writing short replies**
When you write a reply of less than ~5 words, have an alarm bell in the back of your head 🔔. Is it just tacit agreement? Then it’s probably fine! “Great job”, “Couldn’t have put it better”, “toats awesome!” are all fine. In every other case though: 🚨!
## Table of contents

It good practice to elaborate. It may seem inefficient to be so verbose in our replies, but it will avoid misunderstandings, which can result in way greater inefficiency.
- [Short replies](#short-replies)
- [Criticizing or asking questions](#criticizing-or-asking-questions)
- [Giving advice or tips](#giving-advice-or-tips)
- [Clarifications and disagreements](#clarifications-and-disagreements)

## Short replies

When you write a reply of less than ~5 words, have an alarm bell in the back of your head 🔔. Is it just tacit agreement? Then it’s probably fine! “Great job”, “Couldn’t have put it better”, “totes awesome!” are all fine.

In every other case though: 🚨! It's good practice to elaborate. It may seem inefficient to be so verbose in our replies, but it will avoid misunderstandings, which can result in far greater inefficiency.

## Criticizing or asking questions
Copy link
Contributor

Choose a reason for hiding this comment

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

👀 what happened here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🇺🇸 spelling to be internally consistent with everything I was rewriting


## Criticising or asking questions
Start by repeating **what** you are replying to in your own words. This forces you to indulge yourself into the perspective of the other, which creates a greater understanding of the issue both for yourself and the other when reading your reply.

### To be avoided:
### Avoid:

- Don't use optional here.
- What’s your suggestion?

### Preferred:
- **I see you are using an optional as an array of User object in the example above**, have you considered to have that property default to an empty array?
### Prefer:

- **I see you are using an optional as an array of User object in the example above**, have you considered having that property default to an empty array?
- **Okay, so you say using a collectionView here would be over-engineering**, how would you then solve this problem instead?

## Giving advice or tips

Explain why you think this might be helpful in this particular case. Don’t just prescribe, but try to make your case for a particular approach.

### Avoid:

- Why not use a CollectionView for this?
- Read this article:

### Prefer:

- A collection view might be helpful in this case, **because it would take care of the layout for you. Otherwise you’ll be redoing the same work, implementing your own lightweight collection view instead.**
- This article might be interesting for you, **as it explains the threats of storing API keys in plaintext.**

## When you feel someone is not communicating clear enough
If you feel someone’s reaction is not clear enough (i.e. someone’s reaction confuses you, or even agitates you), look at these guidelines together to see what could be improved in the future. If you realise something critical is missing in the guidelines, please add to it.
## Clarifications and disagreements

If you feel someone’s reaction is not clear enough (i.e. someone’s reaction confuses you, or even agitates you), ask for a clarification, or if the person could rephrase that, if the meaning isn’t clear.

Try to keep in mind that written communication it’s really easy to misread something or to interpret something out of context. Try not to assume rudeness, ill intent or malice. Sometimes something was poorly phrased or misread. Sometimes we’re switching between contexts too often, and end up being shorter than we should. Try asking for clarification, or if the person could rephrase that, if the meaning isn’t clear. 👍
Try to keep in mind that in written communication, it’s really easy to misread something or to interpret something out of context. Sometimes something was poorly phrased. Sometimes we’re switching between contexts too often, and end up being shorter than we should.

If following the guidelines don't clear things up, consider a call to solve the issue if the matter is pressing, or wait for the next iOS meeting to continue discussing it.
However, don't be afraid to let your colleagues know if you feel they're not following these guidelines, or if you're feeling attacked. All humans can use reminders from time to time to be a little nicer to each other.

### Good to keep in mind:
Keep in mind that you are often reviewing or discussing something that someone else has worked on really hard, so always be respectful. [These guidelines for giving feedback on pull requests are a good reference](GIT_AND_GITHUB.md#reviewing-pull-requests).

When reviewing pull requests keep in mind that you are reviewing something that someone else has worked on really hard, so always be respectful. These guidelines for giving feedback are a good reference:
If following these suggestions doesn't clear things up, consider a call to solve the issue if the matter is pressing, or wait for the next iOS meeting to continue discussing it.

https://github.com/bakkenbaeck/iOS-playbook/blob/master/GIT_AND_GITHUB.md#reviewing-pull-requests
For the long term, look at this document together to see what could be improved in the future. If you realize something critical is missing in these guidelines, please add to it.
90 changes: 33 additions & 57 deletions GIT_AND_GITHUB.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,13 @@ These conventions apply to our open-source, internal, and client projects.

## Commits

The `master` branch is the stable branch. It's also used for development
purposes, as to avoid having `dev`, `development`, `staging`, `wip` and others.
The `master` branch is the stable branch. It's also used for development purposes, as to avoid having `dev`, `development`, `staging`, `wip` and others.

Choose a reason for hiding this comment

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

👍


You might be wondering: why only `master`? What if I have to rollback to apply
a fix? Well, if you find a bug in a previous release, you can just, checkout the tag,
fork it, fix it, deploy the new build, and finally, submit a PR to merge it with `master`
to the sound of Daft Punk.
You might be wondering: why only `master`? What if I have to rollback to apply a fix? Well, if you find a bug in a previous release, you can just, checkout the tag, fork it, fix it, deploy the new build, and finally, submit a PR to merge it with `master` to the sound of Daft Punk.

Commit messages are in the [imperative present tense](http://stackoverflow.com/questions/3580013/should-i-use-past-or-present-tense-in-git-commit-messages), and have no period at the end.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

note to self for tomorrow: remove this line


Prefer concise commits over gigantic ones. When writing a concise commit message
is difficult, it may indicate too many unrelated changes.

Feel free to elaborate more on the commit description.
Prefer concise commits over gigantic ones. When writing a concise commit message is difficult, it may indicate too many unrelated changes.

```
// Preferred
Expand All @@ -27,11 +20,11 @@ Add employee avatar in list of employees
Added image view to display image for employee in employees table view controller.
```

## Branches
If you need to elaborate more, you can do so in the commit description.

### Naming
## Branches

Branches are awesome. We use `feature/`, `fix/`, `improve/` and `refactor/` for pseudo namespacing:
Branches are awesome. We use `feature/`, `fix/`, `improve/` and `refactor/` for pseudo-namespacing:

```
feature/new-feature
Expand All @@ -40,54 +33,30 @@ improve/make-it-better
refactor/make-it-awesome
```

Sometimes it's okay to just push to `master` if it's a quick fix and you really need
it out the door. Otherwise, make a pull request.
Rarely, it's okay to just push to `master` if it's a quick fix and you really need it out the door. Usually, you make a pull request to `master` or to a long-running `feature` branch instead.

### Pull requests

Pull request descriptions should be concise and well written. The reviewer should
be able to copy this description straight into the release notes, instead of
figuring out what changed or was fixed.
Pull request descriptions should be concise and well written. The reviewer should be able to copy this description straight into the release notes, instead of figuring out what changed or was fixed.

Besides, a description with more information could be helpful, for example:

- If it's a static UI, a screenshot would do.
- If it's an interaction, a gif would help a lot. A good tool to make gifs is [Licecap](http://www.cockos.com/licecap/).
- If it's a static UI, a screenshot will do.
- If it's an interaction, a `gif` would help a lot. Goods tool to make `gif`s include [Licecap](http://www.cockos.com/licecap/), [GIPHY Capture](https://itunes.apple.com/us/app/giphy-capture-the-gif-maker/id668208984?mt=12), and [GIF Brewery](http://gifbrewery.com/).
Copy link
Contributor

Choose a reason for hiding this comment

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

Goods tool, Good tools? 😅

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤦‍♀️

- If it's a bug, steps to reproduce are very useful to help understanding context.

Before merging a pull request, your code has to be reviewed, so that we can learn from you,
point out your silly mistakes, and/or post a sufficient amount of gifs. The reviewing
process is important. It is better for you to have another person backing you up. More eyes
can mean less bugs and more consistency throughout.
Before merging a pull request, your code has to be reviewed, so that we can learn from you, help you find mistakes, and/or post a sufficient amount of gifs. The reviewing process is important. It is better for you to have another person backing you up. More eyes on code mean fewer bugs and better consistency throughout.

#### Code Linting

We try to keep our whitespace and code style consistent, one of the tools we use to achieve this is [swiftformat](https://github.com/nicklockwood/SwiftFormat). We recommend installing it using [homebrew](https://brew.sh), for easy running and updating. For most projects you can run it from the root folder, but for projects using Cocoapods/Carthage, it’s recommended to run it from the top subfolder (usually has the same name as the project), to avoid reformatting third-party code.

#### Reviewing pull requests

As a reviewer, you should ideally be a core team member and have enough context
to make a thorough review of the committed code, just by reading it. However,
there are times when the pull request is a bit on the large side, or it is
referencing existing code not found in the PR, or you for some other reason do
not have a good enough context to review anything else than code style and typos.
In these cases, we encourage you to physically (or virtually) approach the PR
submitter and go through the pull request together.

Be warned, though: By doing this, you will both need to explain your thoughts and
discuss other alternatives. Of course, this means you are putting yourself at risk
of sharing your knowledge and/or learning some new stuff, so please be careful not
to end up being even more awesome than you already are.

As a reviewer your job is to be the extra pair of eyes, so the code will get twice
as good. You'll look at things such as code style and structure. You'll discuss things
that you might've done differently, point out possible more elegant solutions based
on something you've learned, and so on. The goal here is to make the whole team better
in the process.

Keep in mind that you are reviewing something that someone else has worked on really
hard, so always be respectful. These guidelines for giving feedback are a
good reference:
As a reviewer, you should ideally be a core team member and have enough context to make a thorough review of the committed code, just by reading it. However, there are times when the pull request is a bit on the large side, or it is referencing existing code not found in the PR, or you for some other reason do not have a good enough context to review anything else than code style and typos. In these cases, we encourage you to physically (or virtually) approach the PR submitter and go through the pull request together.

Be warned, though: By doing this, you will both need to explain your thoughts and discuss other alternatives. Of course, this means you are putting yourself at risk of sharing your knowledge and/or learning some new stuff, so please be careful not to end up being even more awesome than you already are.

As a reviewer your job is to be the extra pair of eyes, so the code will get twice as good. You'll look at things such as code style and structure. You'll discuss things that you might've done differently, point out possible more elegant solutions based on something you've learned, and so on. The goal here is to make the whole team better in the process.

Keep in mind that you are reviewing something that someone else has worked on really hard, so always be respectful. These guidelines for giving feedback are a good reference:

- Familiarise yourself with the context of the issue.
- If you disagree strongly, consider giving it a few minutes before responding; think before you react. Try to empathise.
Expand All @@ -103,20 +72,27 @@ good reference:

[Based on GitHub's "How to write the perfect pull request".](https://github.com/blog/1943-how-to-write-the-perfect-pull-request)

If everything is fine feel free to merge the pull request. Avoid merging before receiving
a feedback. If several developers are involved in the project, one confirmation should be
enough. You can always submit subsequent pull requests or file an issue after the fact.
If everything is fine, feel free to merge the pull request. Avoid merging before receiving a feedback. If several developers are involved in the project, one confirmation should be enough. You can always submit subsequent pull requests or file an issue after the fact.

After you have merged, you should delete the branch and smile. The smile is critical
part of the process, so don't forget this.
After you have merged, you should delete the branch and smile. The smile is critical part of the process, so don't forget this.
Copy link
Contributor

Choose a reason for hiding this comment

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

…is _a_ critical part… 👍


#### My pull request depends on another pending pull request. What to do?

If your next pull request is really that intimately related to your last,
consider to continue working on it instead of opening another.

If it really makes sense to open another, it's okay to base your next pull
request on your unmerged branch, just make sure to make it clear in the PR description.
Finally make sure to merge any changes you've made to its parent during its review.
If it really makes sense to open another, it's okay to base your next pull request on your unmerged branch, just make sure to make it clear in the PR description. Note in the description that the PR depends on its parent PR, and add a link to the pending parent PR.

Finally, make sure to merge any changes you've made to its parent during its review before seeking final review of your child PR.

[_This is partly based on Hyper's Git and GitHub guide._](https://github.com/hyperoslo/playbook/blob/master/GIT_AND_GITHUB.md)

## Releases

Steps for releasing a new version of your app:

1. Switch to the `master` branch.
2. Bump the projects version number (make sure to use [semantic versioning](http://semver.org/)) and build number.
3. Create the archive.
4. Upload the archive to iTunes Connect for submission to TestFlight or App Review.
4. Create a [release on GitHub](https://help.github.com/articles/creating-releases/). Remember to mark it as a `pre-release` if needed.
Loading