-
Notifications
You must be signed in to change notification settings - Fork 202
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
GitHub script2.0 #96
base: main
Are you sure you want to change the base?
GitHub script2.0 #96
Conversation
Just update the read me. Will start to build these function.
All method but create teams worked.
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.
this is a good start, but i made a number of comments regarding clarity especially in the documentation. also, i'm really concerned that there is "special case" code for Fa23 only that will have to be removed later, and that the documentation has conflicting info about what happens in this case (when script is re-run and student team already exists). this should really be done in a cleaner way, such as providing an option that specifies what to do in each case.
scripts/github-repos/README.md
Outdated
|
||
``` | ||
Usage: ./github-repos.rb [required options] [invite|repos|remove|remove_access] | ||
Usage: ./github-repos.rb [required options] [invite|create_teams|indiv_repos|team_repos|remove_indivi_repos|remove_team_repos|remove_teams|remove_team_repo_access] [optional options] |
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.
are you sure no typos in the list of all possible commands?
scripts/github-repos/README.md
Outdated
-t, --template=TEMPLATE The repo name within the org to use as template eg repo_name (Assume the repo own by org) | ||
'invite': | ||
Create a team called STUDENTTEAM under the org that has all students in the same semester and send invitation | ||
If STUDENTTEAM is already exist, the script will resent invitation to students. |
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.
"already exists", and "resend"
scripts/github-repos/README.md
Outdated
'invite': | ||
Create a team called STUDENTTEAM under the org that has all students in the same semester and send invitation | ||
If STUDENTTEAM is already exist, the script will resent invitation to students. | ||
(Temporary for special situations in Fa23) |
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.
why is this temporary? isn't this always the desired behavior? i really don't want to have changes in a script that are there for one semester only. let's discuss what special case is being addressed here and whether it can be addressed without creating a change now that will HAVE to be undone after the semester, since the rest of us have to live with these scripts after you're gone....
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.
Sorry for the confusion here. The temporary means the optional prefix option. In the future, I believe we will not have child teams at the first few weeks. Child teams are for chip10.5 only. So, the invite command should invite students to the STUDENTTEAM rather than create child teams and invite them to those child teams.
If the prefix option is provided, the scripts will assume there is 'Team' column in csv file, creates child teams, and invite students to child teams.
-s, --studentteam=STUDENTTEAM The team name of all the students team | ||
-o, --orgname=ORGNAME The name of the org | ||
|
||
Optional options: |
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.
for optional options, you have to state what the default is if the option is not given
scripts/github-repos/README.md
Outdated
Optional options: | ||
-p, --prefix=PREFIX Semester prefix, eg fa23. | ||
|
||
'create_teams' (Not functioning) |
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.
what does "Not functioning" mean in this case?
scripts/github-repos/README.md
Outdated
``` | ||
|
||
This script creates a team that include all stundents in the same semester, eg fa23. | ||
Then creates different child teams for them for the chip10.5. At a minimum, | ||
you need a CSV file listing all enrolled students with columns 'Email' and 'Team'. | ||
you need a CSV file listing all enrolled students with columns 'Username' and 'Team'. |
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.
does the Username column need to have the student's personal GitHub username, or something else?
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 Username column need to have the student's personal GitHub username. Since student will have emails other than berkerley email associated with GitHub. I think using GItHub username is better to invite students to org.
scripts/github-repos/README.md
Outdated
to the students in csv file. If STUDENTTEAM exists, delete the team and | ||
create a new one. (Team repos still exist) | ||
For all students NOT in that team, add/invite into that team. | ||
to the students in csv file. If STUDENTTEAM exists, delete STUDENTTEAM and |
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.
this contradicts the comments near the beginning of the README about what happens if the team exists. which is correct??
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 will delete this. I believe if the STUDENTTEAM exists, sending invitation again is more reasonable.
scripts/github-repos/README.md
Outdated
|
||
**Use case:** Create CHIP repos for each student in STUDENTTEAM. | ||
The repos' names are formed like "fa23-[username]-FILENAME". There will | ||
be only one collabrator. Add all repos to the gsiteam with admin permission. |
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.
"Give GSITEAM admin access to the created repos."
scripts/github-repos/README.md
Outdated
by concatenating the prefix, base file name, and the team ID. | ||
(eg "fa23-actionmap-04" for team #4) For all students on that team who do | ||
NOT already have access to the repo, give them write access on the repo. | ||
gets a repo for chip 10.5. Repos' names are formed like "PREFIX-FILENAME-TEAMNUM" |
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.
again, this is confusing because you are using "TEAM" to mean both "a group of people on github in a single github team or child team" and "a small group of students working on a single repo/app". it might be less confusing to use "GROUP" to mean the second thing since github already uses "TEAM" for the first.
scripts/github-repos/github-repos.rb
Outdated
$opts.parse! | ||
command = ARGV.pop | ||
case command | ||
when 'invite' then org.invite |
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'd suggest rewriting this code along the following lines. define a class method in OrgManager
that returns a valid list of subcommand names (invite, create_teams, etc) as a list of strings or symbols. then this code can just say:
if OrgManager.commands.include?(command)
org.send(command)
else
org.print_error
end
scripts/github-repos/github-repos.rb
Outdated
@client.add_collab(new_repo['full_name'], mem.login) | ||
@client.add_team_repository(gsiteam_id, new_repo['full_name'], {permission: 'admin'}) |
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 it's best to move this outside the if-statement started on line 287 -- that way, if something happens that prevents the collaborators from being added, we can re-run the script and they can get added to an existing repository the next time.
(I ran into this just now with chips 4.6 -- for some reason a student didn't end up assigned to their repo and it needed a re-run)
I decide to keep the temporary option which creates groups in invite command.
change -perm to -e. type error on opts. syntax error in repos_valid. update the private property of repos if they had existed.
GitHub Script modifications:
For each individually-submitted assignment
Make 1 repo per student, which starts from the public template
fa23-[username]-chips-[number] (can create by using PREFIX-[username]-FILENAME)
Each repo has only 1 collaborator.
Username is the prefix to the berkeley.edu email address (e.g. “ball” for “[email protected]”)
Github API doesn't provide adding collaborator using emails, and it also doesn't provide a way to get Github users emails.
Students also have emails other than berkeley email associated with Github account. Therefore, it is difficult to create repos and add students as collaborator to the repos using berkeley emails.
In this script, it will create repos using Github name for students in the students team. (Whether students are in the child teams for 10.5)