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

Updated all referenecs to old github repo #479 #520

Merged
merged 2 commits into from Sep 2, 2019

Conversation

@review-notebook-app

This comment has been minimized.

Copy link

commented Sep 1, 2019

Check out this pull request on ReviewNB: https://app.reviewnb.com/skorch-dev/skorch/pull/520

You'll be able to see notebook diffs and discuss changes. Powered by ReviewNB.

@joshy joshy referenced this pull request Sep 1, 2019
@BenjaminBossan

This comment has been minimized.

Copy link
Collaborator

commented Sep 1, 2019

Thanks for this PR. Looks almost good to go. A few comments:

  • I think you forget one reference in index.rst (this is what I found with grep).
  • You changed some newlines, was that on purpose?
  • You changed the references in CHANGES.md; I wonder if one should retroactively change the CHANGES file, not sure if there is any disadvantage in that (ping @ottonemo @thomasjpfan).
@joshy

This comment has been minimized.

Copy link
Contributor Author

commented Sep 1, 2019

Thanks, about your comments:

  • reference in index.rst points to https://github.com/dnouri/nolearn which has not been moved
  • That was not on purpose, I will put them back in (accidentally committed)
  • About CHANGES.md: Whatever you/team decides
@thomasjpfan

This comment has been minimized.

Copy link
Member

commented Sep 2, 2019

I wonder if one should retroactively change the CHANGES file, not sure if there is any disadvantage in that

The changes are links that redirect to this repo, so I think it is okay.

@BenjaminBossan BenjaminBossan self-requested a review Sep 2, 2019

@BenjaminBossan
Copy link
Collaborator

left a comment

Thank you very much, looks good to me.

@BenjaminBossan BenjaminBossan merged commit 066eb39 into skorch-dev:master Sep 2, 2019

2 checks passed

Travis CI - Pull Request Build Passed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.