Skip to content
This repository has been archived by the owner on Apr 27, 2023. It is now read-only.

Add support for teams #60

Merged
merged 3 commits into from
Feb 29, 2016
Merged

Add support for teams #60

merged 3 commits into from
Feb 29, 2016

Conversation

gerits
Copy link
Contributor

@gerits gerits commented Feb 3, 2016

We are developing an application with about 30 developers that all contribute in the same repository. It's not interesting to review everybody's code, so I've added a team functionality.

It allows to create teams and add existing users to this team.
A team member can either be a spectator or a contributor. The commits of all the contributors are listed in the "to review" tab. Being part of a team as spectator allows you to view and comment, but other members won't be able to see your commits.

These are my initial requirements and I've limited my implementation to this. I was not sure yet on how to display it in the ui and what to do as default behavior for users that are not part of a team (nothing to review?).

Does a functionality like this require a more advanced user system or can it exist separate from the user system as I've implemented it now.

@bazhe
Copy link

bazhe commented Feb 4, 2016

This is really one interest feature, I will waiting for your merge!

@@ -4,4 +4,4 @@ cd codebrag-ui
nohup npm install && ./node_modules/.bin/grunt server &

cd ../
java -Dfile.encoding=UTF8 -Xmx3000M -Xss1M -XX:+CMSClassUnloadingEnabled -XX:MaxPermSize=1024m -jar sbt-launch.jar "~ container:start"
java -Dfile.encoding=UTF8 -Xmx1590M -Xss1M -XX:+CMSClassUnloadingEnabled -XX:MaxPermSize=1024m -jar sbt-launch.jar "~ container:start"
Copy link
Member

Choose a reason for hiding this comment

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

Why did you reduce this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was developing on a 32-bit machine, this has a memory limitation. But this should indeed not be part of the merge. I will revert this.

@lukaszlenart
Copy link
Member

I'm wondering how to treat teamless users, showing nothing to review isn't the best idea. I would rather show everything, at the end a team is yet another filter. It would be good to have a switch to disable team filter but that can be added latter

teamFinder.findAllAsManagedTeamMembers(team)
} else {
ManagedTeamMembersListView(List.empty)
}
Copy link
Member

Choose a reason for hiding this comment

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

Could you revert this if clause?

if (config.demo) {
  ManagedTeamMembersListView(List.empty)
} else {
  var team = teamFinder.findTeam(new ObjectId(params("teamId")));
  teamFinder.findAllAsManagedTeamMembers(team)
}

@gerits
Copy link
Contributor Author

gerits commented Feb 9, 2016

The filtering of the team members seemed logical at first for an all team setup. But it indeed requires some visualisation:

  • By default show everything.
  • When in a team there should be somewhere an indication of which team (profile page?).
  • Extra filter needed?: to review team members, to review all, all commits
  • Possibility to request to join a team or fully managed by admin.

@lukaszlenart
Copy link
Member

Let's start with the simplest option - user is not assigned to any team, show everything

- cleanup
- show all users to review if not part of team
@lukaszlenart
Copy link
Member

Is it ready for final shot?

@gerits
Copy link
Contributor Author

gerits commented Feb 25, 2016

We have been using it for the past weeks now, and did not see any problems. Although addition features could be added, this is a stable first version of the 'teams' support.

@lukaszlenart
Copy link
Member

Gut! Waiting for final confirmation from teammates and

lukaszlenart added a commit that referenced this pull request Feb 29, 2016
@lukaszlenart lukaszlenart merged commit 7b0a549 into softwaremill:master Feb 29, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants