Skip to content

Add --team flag to download command#652

Merged
kytrinyx merged 3 commits intomasterfrom
teams-download
Jul 26, 2018
Merged

Add --team flag to download command#652
kytrinyx merged 3 commits intomasterfrom
teams-download

Conversation

@kytrinyx
Copy link
Copy Markdown
Member

@kytrinyx kytrinyx commented Jul 18, 2018

We're working on implementing teams, which is pretty different from how it worked on the old site. In short, teams are a whole separate thing. If you are working on bob in Python on the main site, and you're a member of a team, then your bob does not automatically end up in the team pool of solutions. You need to explicitly submit it with a --team flag (not yet implemented in the CLI). Likewise, to work on an exercise for a team, you need to download it so that you get the correct metadata, then you can copy over your solution from your main track.

Closes #546

@kytrinyx kytrinyx requested a review from nywilken July 18, 2018 02:20
Comment thread cmd/download.go

dir := usrCfg.GetString("workspace")
if solution.Team != "" {
dir = filepath.Join(dir, "teams", solution.Team)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is a bit confusing. In payload it is a struct here Team is a string. I understand the difference but maybe it should be TeamSlug. Or maybe have a different name in the Solution struct

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah, I think TeamSlug makes sense.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I made this change.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I changed it back, actually.

In the solution we're storing just the minimal metadata. The payload has more data about both the track and the team, which we can use to communicate about it if we want to.

I would rather change the solution type later to have a more fleshed out team than to call this TeamSlug now.

Copy link
Copy Markdown
Contributor

@nywilken nywilken left a comment

Choose a reason for hiding this comment

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

I started looking through this and structurally it makes sense. It needs to be rebased with the latest changes made to the download command. I took a bit longer with the other PRs so I will get to this one later today.

@kytrinyx
Copy link
Copy Markdown
Member Author

Yeah, I submitted small parts of this as other PRs, so I need to rework it so it's just the smallest possible change.

@kytrinyx kytrinyx changed the title Add --team flag to download command [WIP] Add --team flag to download command Jul 24, 2018
@kytrinyx kytrinyx changed the title [WIP] Add --team flag to download command Add --team flag to download command Jul 24, 2018
@kytrinyx
Copy link
Copy Markdown
Member Author

I think this is ready for another round of review. I've cleaned it up and the PR is as small as I can get it and still be a coherent change :)

@kytrinyx
Copy link
Copy Markdown
Member Author

kytrinyx commented Jul 24, 2018

Wait. The first two commits are in a separate PR already (#676). Hold off on reviewing this until the other PR is merged, then I'll rebase and this will be smaller.

@kytrinyx kytrinyx changed the title Add --team flag to download command [WIP] Add --team flag to download command Jul 24, 2018
@nywilken
Copy link
Copy Markdown
Contributor

If you are working on bob in Python on the main site, and you're a member of a team, then your bob does not automatically end up in the team pool of solutions. You need to explicitly submit it with a --team flag (not yet implemented in the CLI). Likewise, to work on an exercise for a team, you need to download it so that you get the correct metadata, then you can copy over your solution from your main track.

So If I understand correctly, you can not submit a solution to your team unless you download it with the team flag. Is that correct? If so do we need to handle the case where a user is trying to submit for a team but has none.

If that is the case what should be the expected use case when I do the following

exercism d --track go --exercise hello-word --team bar
cd $(exercism workspace)/go/hello-world; exercism s hello.go

Since the team flag is not provided should it remove the team property from the solution file?

@kytrinyx kytrinyx force-pushed the teams-download branch 2 times, most recently from 821d5ec to b268ce8 Compare July 25, 2018 21:40
@kytrinyx kytrinyx changed the title [WIP] Add --team flag to download command Add --team flag to download command Jul 25, 2018
@kytrinyx
Copy link
Copy Markdown
Member Author

Ok, merged the previous PR and rebased this one. Should be good to review!

Katrina Owen added 3 commits July 25, 2018 15:50
This updates the fake server to fill in the teams in the payload if
the team flag is passed.

Nothing is passing the team flag yet.
Comment thread workspace/solution.go
Track string `json:"track"`
Exercise string `json:"exercise"`
ID string `json:"id"`
Team string `json:"team,omitempty"`
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

what is the intended empty state for Team “”, null? I ask because I am wondering if this should be of type *string.

If the empty state is “” then your example of “null” in the test should be changed.

Copy link
Copy Markdown
Member Author

@kytrinyx kytrinyx Jul 26, 2018

Choose a reason for hiding this comment

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

The empty state of the solution metadata is "". The empty state of the payload we receive from the server is null.

Comment thread cmd/download_test.go

payloadBody := fmt.Sprintf(payloadTemplate, requestor, server.URL+"/")
mux.HandleFunc("/solutions/latest", func(w http.ResponseWriter, r *http.Request) {
team := "null"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

See my comment about the Team field for the Solution type.

Copy link
Copy Markdown
Contributor

@nywilken nywilken left a comment

Choose a reason for hiding this comment

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

This looks good in its current state. I was concerned about the data, but since it is being controlled via the API we don’t have to account for edge cases in the data, but we can easily adapt if needed.

@kytrinyx
Copy link
Copy Markdown
Member Author

Thank you!

@kytrinyx kytrinyx merged commit 56eda46 into master Jul 26, 2018
@kytrinyx kytrinyx deleted the teams-download branch July 26, 2018 18:00
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

Successfully merging this pull request may close these issues.

2 participants