-
Notifications
You must be signed in to change notification settings - Fork 6
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
Issue #13 - Update Readme #43
Conversation
Bagurp
commented
Jun 16, 2020
- Updates README.md
- Adds CONTRIBUTING.md
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.
I don't think these edits ended up as intensive as I thought they might. Sorry for being slow on the PR!
|
||
This project uses a [trunk based development](https://trunkbaseddevelopment.com/) branching strategy. | ||
|
||
[Trunk-based Development vs. Git Flow](https://www.toptal.com/software/trunk-based-development-git-flow) |
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.
So, unless the team objects, I don't think we want to use trunk-based development for this project. There will potentially be people coming on and off frequently with varying skill levels, necessitating PRs to keep our codebase from going in multiple directions. That's just my $0.02 though. I'd love to hear the thoughts of the team on this.
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.
I agree, I think git-flow development model suits this project. I could use this wiki in the documentation - https://www.atlassian.com/git/tutorials/comparing-workflows/gitflow-workflow
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.
I think we can all agree git-flow
is the preferred branching strategy.
@Bagurp Could you change line 21 to read:
This project uses a [git-flow based development](https://www.atlassian.com/git/tutorials/comparing-workflows/gitflow-workflow) branching strategy.
?
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.
Updated text. Should be resolved with the new changes.
CONTRIBUTING.md
Outdated
|
||
## Branches | ||
<a id="markdown-branches" name="branches"></a> | ||
*Branch naming must be consistent. Additonal patterns above case may be added as necessary. Automated enforcement in CI is recommended.* |
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.
Let's remove "Additonal patterns above case may be added as necessary. Automated enforcement in CI is recommended." I'd add the following patterns as well...open to differing suggestions from the team:
feature/* - Feature additions
bugfix/* - Bug fixes
chore/* - Chore tasks
doc/* - Documentation tasks
Anything I'm missing?
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.
If everyone agrees then I think adding story# to the branch name works well. for example: feature/S13-update-readme.
Also, we may want to add "release/*" for release branches.
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.
Not sure we will have the need for release branches for a while, but I'm ok with adding it.
I'm down with adding issue number as well, but I tend to prefer having it included in the prefix: feature-13/update-readme
. If the rest of the team prefers something else though, I really don't have a strong opinion on it. It's just my familiarity bias. ;-)
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.
I am okay either way. At the end of the day they need to be able to be merged.
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.
Updated text. Should be resolved with the new changes.
## Commits | ||
<a id="markdown-commits" name="commits"></a> | ||
*Consistent, verbose commits reduce the need for additional documentation. May be used for generating changelogs. Detailed instructions must be included either through a documented specification or custom documentation. Automated enforcement in CI is recommended.* | ||
|
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.
Let's remove "Automated enforcement in CI is recommended." from all of these bolded statements. Those are just suggestions for Innovation Lab projects. I'm happy if we implement some of those recommended enforcements, but until we have there's no point in putting those suggestions in these docs, IMHO. They just point out what we haven't gotten around to yet.
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.
@Bagurp can you remove the last sentence and them @mkimberlin can resolve.
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.
Updated text. Should be resolved with the new changes.
<a id="markdown-task-workflow" name="task-workflow"></a> | ||
*Description of how tasks are tracked through project management software and expectations of different roles.* | ||
|
||
 |
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.
The image is missing and needs to be copied over.
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.
@Bagurp has this been updated or resolved?
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.
Added the image to repo. Should be resolved with the new changes.
@mkimberlin once the changes to the Contributing documentation are made, could you please remove the |
706c421
to
3a9872e
Compare