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

sync: add flag providing path to non-transient problem-specifications repo #47

Closed
coriolinus opened this issue Oct 16, 2020 · 6 comments · Fixed by #65
Closed

sync: add flag providing path to non-transient problem-specifications repo #47

coriolinus opened this issue Oct 16, 2020 · 6 comments · Fixed by #65
Assignees
Labels
cmd: sync kind: feature User-facing enhancement

Comments

@coriolinus
Copy link
Member

It's cool that, left to its own devices, the canonical data syncer shallow-clones the problem-specifications repo into a temporary folder, then cleans up after itself. However, this is somewhat wasteful of bandwidth, even though it's normally fast.

If someone has a permanent problem-specifications repo, then it would be nice if they could provide its location to the syncer, giving the syncer permission to do essentially:

cd "$problem_specifications"
git checkout -- . # clear out any temporary modifications
git checkout master # or main, whatever,
git pull
@ee7
Copy link
Member

ee7 commented Oct 16, 2020

I agree that this could be nice, even if just for responsiveness. It currently just does the simplest thing that will always work.

Quickly testing on a not-so-great connection:

git clone --depth 1 https://github.com/exercism/problem-specifications.git 

takes about 1.8 s, compared to about 0.7 s for a git pull.

git checkout -- . # clear out any temporary modifications

This silently and non-interactively discards changes to the working directory, which we probably don't want to do even the location is passed explicitly - people could lose uncommitted changes.

We could implement it with interaction, e.g.:

[problem-specifications working directory] has uncommitted changes, discard them and continue anyway? [y/N]
y
You're going to lose these changes, are you really sure? [y/N]
[insert git diff]
y

But perhaps a good first implementation of this could just fail if we couldn't git checkout master.

@coriolinus
Copy link
Member Author

But perhaps a good first implementation of this could just fail if we couldn't git checkout master.

Sure, fair enough.

@ErikSchierboom
Copy link
Member

I like this suggestion.

@glennj
Copy link
Contributor

glennj commented Oct 16, 2020

Or, instead of git checkout -- . we could do git stash

Or, instead of fiddling with the user's problem-spec repo, just give them a message if they're not on the master branch and not at the remote HEAD.

@ErikSchierboom
Copy link
Member

Being able to specify the path to a prob-specs repo also allows us to finally add some tests. See #4. If anyone is up for adding some integration tests (which should work with the actual binary), that would be fantastic.

@ee7 ee7 self-assigned this Oct 20, 2020
ee7 added a commit to ee7/exercism-configlet that referenced this issue Oct 21, 2020
The user can now tell the program to use an existing
`problem-specifications` directory, in which case it doesn't make a
temporary shallow clone.

This saves a significant amount of time, and is probably the biggest
possible speed optimisation given that the program running time in
non-interactive mode is essentially the time it takes to establish an
up-to-date `problem-specifications` directory. But perhaps we could
consider adding an "offline-mode" in the future.

This commit also makes it easier to add tests.

Decisions:
- Show an error if the given `problem-specifications` working directory
  is not clean, rather than asking the user or stashing changes.

Closes: exercism#47
ee7 added a commit to ee7/exercism-configlet that referenced this issue Oct 21, 2020
The user can now tell the program to use an existing
`problem-specifications` directory, in which case it doesn't make a
temporary shallow clone.

This saves a significant amount of time, and is probably the biggest
possible speed optimisation given that the program running time in
non-interactive mode is essentially the time it takes to establish an
up-to-date `problem-specifications` directory. But perhaps we could
consider adding an "offline-mode" in the future.

This commit also makes it easier to add tests.

Decisions:
- Show an error if the given `problem-specifications` working directory
  is not clean, rather than asking the user or stashing changes.

Closes: exercism#47
@ErikSchierboom
Copy link
Member

@coriolinus @glennj There is a related issue opened in #66 and I've added some comments. I'd be interested in hearing your opinions.

ee7 added a commit to ee7/exercism-configlet that referenced this issue Oct 22, 2020
The user can now tell the program to use an existing
`problem-specifications` directory, in which case it doesn't make a
temporary shallow clone.

This saves a significant amount of time, and is probably the biggest
possible speed optimisation given that the program running time in
non-interactive mode is essentially the time it takes to establish an
up-to-date `problem-specifications` directory. But we could add an
"offline" option in the future.

This commit also makes it easier to add tests.

Decisions:
- Show an error if the given `problem-specifications` working directory
  is not clean, rather than asking the user or stashing changes.
- Try to be strict about the user's `problem-specifications` directory,
  and do not silently `checkout` or `merge`.
- Don't assume the user has a remote named 'upstream'.
- Allow the given `problem-specifications` repo to be a shallow clone:
  don't check that it's actually a `problem-specifications` repo by
  requiring it to contain some specified SHA-1.

Closes: exercism#47
ee7 added a commit to ee7/exercism-configlet that referenced this issue Oct 23, 2020
The user can now tell the program to use an existing
`problem-specifications` directory, in which case the program does a
`fetch` rather than making a temporary shallow clone.

This is one of the biggest possible speed optimisations, given that the
program running time in non-interactive mode is limited by the time
taken to establish an up-to-date `problem-specifications`directory.
However, in the future we could add an `--offline` option (or similar).

This commit also makes it easier to add tests.

Decisions:
- Show an error if the given `problem-specifications` working directory
  is not clean, rather than asking the user or stashing changes.
- Try to be strict about the user's `problem-specifications` directory,
  and do not silently `checkout` or `merge`, at least for now.
- Don't assume the user has a remote named 'upstream'.
- Allow the given `problem-specifications` repo to be a shallow clone:
  don't check that it's actually a `problem-specifications` repo by
  requiring it to contain some specified SHA-1.

Closes: exercism#47
ee7 added a commit to ee7/exercism-configlet that referenced this issue Oct 23, 2020
The user can now tell the program to use an existing
`problem-specifications` directory, in which case the program does a
`fetch` rather than making a temporary shallow clone.

This is one of the biggest possible speed optimisations, given that the
program running time in non-interactive mode is limited by the time
taken to establish an up-to-date `problem-specifications`directory.
However, in the future we could add an `--offline` option (or similar).

This commit also makes it easier to add tests.

Decisions:
- Show an error if the given `problem-specifications` working directory
  is not clean, rather than asking the user or stashing changes.
- Try to be strict about the user's `problem-specifications` directory,
  and do not silently `checkout` or `merge`, at least for now.
- Don't assume the user has a remote named 'upstream'.
- Allow the given `problem-specifications` repo to be a shallow clone:
  don't check that it's actually a `problem-specifications` repo by
  requiring it to contain some specified SHA-1.

Closes: exercism#47
ee7 added a commit to ee7/exercism-configlet that referenced this issue Oct 23, 2020
The user can now tell the program to use an existing
`problem-specifications` directory, in which case the program does a
`fetch` rather than making a temporary shallow clone.

This is one of the biggest possible speed optimisations, given that the
program running time in non-interactive mode is limited by the time
taken to establish an up-to-date `problem-specifications`directory.
However, in the future we could add an `--offline` option (or similar).

This commit also makes it easier to add tests.

Decisions:
- Show an error if the given `problem-specifications` working directory
  is not clean, rather than asking the user or stashing changes.
- Try to be strict about the user's `problem-specifications` directory,
  and do not silently `checkout` or `merge`, at least for now.
- Don't assume the user has a remote named 'upstream'.
- Allow the given `problem-specifications` repo to be a shallow clone:
  don't check that it's actually a `problem-specifications` repo by
  requiring it to contain some specified SHA-1.

Closes: exercism#47
ee7 added a commit to ee7/exercism-configlet that referenced this issue Oct 23, 2020
The user can now tell the program to use an existing
`problem-specifications` directory, in which case the program does a
`fetch` rather than making a temporary shallow clone.

This is one of the biggest possible speed optimisations, given that the
program running time in non-interactive mode is limited by the time
taken to establish an up-to-date `problem-specifications`directory.
However, in the future we could add an `--offline` option (or similar).

This commit also makes it easier to add tests.

Decisions:
- Show an error if the given `problem-specifications` working directory
  is not clean, rather than asking the user or stashing changes.
- Try to be strict about the user's `problem-specifications` directory,
  and do not silently `checkout` or `merge`, at least for now.
- Don't assume the user has a remote named 'upstream'.
- Allow the given `problem-specifications` repo to be a shallow clone:
  don't check that it's actually a `problem-specifications` repo by
  requiring it to contain some specified SHA-1.

Closes: exercism#47

Co-authored-by: Erik Schierboom <[email protected]>
ErikSchierboom added a commit that referenced this issue Oct 23, 2020
The user can now tell the program to use an existing
`problem-specifications` directory, in which case the program does a
`fetch` rather than making a temporary shallow clone.

This is one of the biggest possible speed optimisations, given that the
program running time in non-interactive mode is limited by the time
taken to establish an up-to-date `problem-specifications`directory.
However, in the future we could add an `--offline` option (or similar).

This commit also makes it easier to add tests.

Decisions:
- Show an error if the given `problem-specifications` working directory
  is not clean, rather than asking the user or stashing changes.
- Try to be strict about the user's `problem-specifications` directory,
  and do not silently `checkout` or `merge`, at least for now.
- Don't assume the user has a remote named 'upstream'.
- Allow the given `problem-specifications` repo to be a shallow clone:
  don't check that it's actually a `problem-specifications` repo by
  requiring it to contain some specified SHA-1.

Closes: #47

Co-authored-by: Erik Schierboom <[email protected]>
ee7 added a commit to ee7/exercism-configlet that referenced this issue Jan 21, 2021
The user can now tell the program to use an existing
`problem-specifications` directory, in which case the program does a
`fetch` rather than making a temporary shallow clone.

This is one of the biggest possible speed optimisations, given that the
program running time in non-interactive mode is limited by the time
taken to establish an up-to-date `problem-specifications`directory.
However, in the future we could add an `--offline` option (or similar).

This commit also makes it easier to add tests.

Decisions:
- Show an error if the given `problem-specifications` working directory
  is not clean, rather than asking the user or stashing changes.
- Try to be strict about the user's `problem-specifications` directory,
  and do not silently `checkout` or `merge`, at least for now.
- Don't assume the user has a remote named 'upstream'.
- Allow the given `problem-specifications` repo to be a shallow clone:
  don't check that it's actually a `problem-specifications` repo by
  requiring it to contain some specified SHA-1.

Closes: exercism#47

Co-authored-by: Erik Schierboom <[email protected]>
@ee7 ee7 changed the title CLI feature request: flag providing path to non-transient problem-specifications repo sync: add flag providing path to non-transient problem-specifications repo May 10, 2021
@ee7 ee7 added cmd: sync kind: feature User-facing enhancement labels May 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cmd: sync kind: feature User-facing enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants