search

strongswan / strongTNC

BYOD TNC Database Management Tool
GNU Affero General Public License v3.0
21 stars 10 forks source link

First bugfixes and refactorings #2

Closed d22 closed 10 years ago

d22 commented 10 years ago

As discussed at the meeting (19. Mar. 14).

tobiasbrunner commented 10 years ago

It might be better to use separate branches in your repository for pull requests. Otherwise, you'll have to do non-fast-forward updates of your master branch, if changes are required (see my comments). Updating to the upstream master is also trickier this way (unless you want to merge back and forth, which results in a really ugly history quite quickly). I usually rebase the master branch in my forks to the upstream master and do my changes in separate branches.

Also, please update the commit messages so that they all start with a capital letter, are not too long and don't contain smileys :).

I'm also not sure if referring to issues in your repository makes sense (at least in the earlier commits). And rebasing the pull requests that happened in your repository could also result in a cleaner history (so we don't merge your merges).

dbrgn commented 10 years ago

@tobiasbrunner see git and workflow sections in https://github.com/tnc-ba/stuff/blob/6036eddb933ee4d6fe3f69e98033825194e49da5/guidelines.pdf?raw=true. Do they make sense?

We'll rebase -i this pull request accordingly, probably on wednesday.

tobiasbrunner commented 10 years ago

I still think you should not push your changes directly to your master branch. But instead maintain a separate integration branch that you then use as source for the next pull request against the main repository. Or you create such pull requests for your individual feature branches.

The Git guidelines look fine. Referring to tickets in commit messages is OK as long as the repository is included in the reference, but doing so in the first line of the of the message is probably not such a good idea (due to the length limit).

dbrgn commented 10 years ago

We discussed this, and we'll continue to use the master branch for development, but we'll create a "snapshot" branch for each pull request against upstream.

Good point about fully qualified issue references.

Please close this pull request for now. We'll create a new one.