Skip to content
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

Add masterAdm command #33

Closed
wants to merge 1 commit into from
Closed

Add masterAdm command #33

wants to merge 1 commit into from

Conversation

eloots
Copy link
Collaborator

@eloots eloots commented Apr 10, 2017

  • This initial version of masterAdm add support for exercise
    renumbering in a course master repository

- This initial version of masterAdm add support for exercise
  renumbering in a course master repository
@@ -595,6 +595,32 @@ Untracked files:
nothing added to commit but untracked files present (use "git add" to track)
```

### masterAdm
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand the naming of the command. Why not just call it "renumber"?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Because I think there will be other features that will be added to this single master admin command which will be selectable (and perhaps combined with) form a list of features.

Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be better to either a) Implement those other features using different commands, or b) Deal with that problem once it comes up?

Just concerned that right now all we need is a renumber so why not call it that. Later as new things are required we can look at introducing them and how to introduce them, but for now we don't have a use case.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have a few things in mind, but wanted to get started, hence the name of this command. Other things:

  • Delete a range or ranges of exercises
  • Create a gap of a given size @ a certain exercise number
  • Create a gap @ a certain exercise and duplicate the code from previous or next exercise.
  • ...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@WadeWaldron @rgcase Wanted to keep things as simple as possible in this version. The general idea behind this implementation was to be able to delete exercises:

  • Delete exercise_xxx_* folders and just run masterAdm -r
  • Add exercises by creating gaps where needed, add new exercise folders at the appropriate location and run masterAdm -r
    That would do the trick for adding and deleting exercises.
    Can create commands as requested for sure, but wanted to spend time on other things first.

@rgcase
Copy link
Contributor

rgcase commented Apr 10, 2017

I'm curious why you didn't just implement the command to do "delete exercise_XYZ" and "insert gap after exercise_XYZ". This seems more complicated and requires an extra renumbering step when you're finished.

val exercises: Vector[String] = getExerciseNames(masterRepo)
val ExerciseNameSpec(_, firstExerciseNr, _) = exercises.head
val (exercisesRenumbered: Vector[String], _) =
(exercises foldLeft(Vector.empty[String], renumberOffset)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe I don't understand but this seems to be pushing the first exercise up by renumberOffset then adding gaps of size renumberDelta. I thought you meant to only start the gaps at index renumberOffset?

If the former is what you want, then can't we just skip the renumberOffset and offset everything by renumberDelta?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, that would be possible too. The idea behind renumberOffset was to be able to add exercises in front of the first exercise. Of course, just using renumberDelta and starting the first renumbered exercise at renumberDelta instead of 0 would sort of achieve the same result.

@eloots eloots mentioned this pull request Apr 26, 2017
@eloots
Copy link
Collaborator Author

eloots commented Dec 21, 2017

Superseded by PR #55.

@eloots eloots closed this Dec 21, 2017
@eloots eloots deleted the add-masterAdm-cmd branch April 29, 2019 13:48
eloots pushed a commit that referenced this pull request May 4, 2022
* removed the explicit strings from the test specs

* refactored into packages

* refactored client to match admin test packages

* added outline client cli command specs (#34)
This pull request was closed.
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.

3 participants