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

Documentation changes related to Git migration #50

Merged
merged 3 commits into from
Aug 4, 2015

Conversation

kkm000
Copy link
Contributor

@kkm000 kkm000 commented Aug 4, 2015

Added Git tutorial, removed Subversion tutorial and updated multiple references from Subversion to Git and from SourceForge to Kaldi's own web site or GitHub as appropriate.

Please comment on particular issues in-line on GitHub page as the change list is large.

EDIT: Many whitespace changes here because of my emacs setup. Add ?w=0 to the end of the URL to show only non-whitespace changes, e. g.

https://github.com/kaldi-asr/kaldi/pull/50/files?w=0

…l and updated multiple references from Subversion to Git and from SourceForge to Kaldi's own web site or GitHub as appropriate.
@danpovey
Copy link
Contributor

danpovey commented Aug 4, 2015

Thanks.. couple of points;
regarding dependencies, we still depend on svn being installed to install
some other stuff.

Regarding this, "
We still intend to maintain a+ read-only Subversion mirror of the GitHub
parent, located at SourceForge and mirrored+ by us."
this is no longer the plan, because Sourceforge lost our latest svn commits
and if we attempt to keep the SF up to date, existing SF users would likely
get corrupted copies of Kaldi.
There is a todo in your commit:
+\todo This paragraph is full of lies!
it would be good if you can find time to fix this before we merge the pull
request.
Dan

On Mon, Aug 3, 2015 at 8:37 PM, Kirill Katsnelson notifications@github.com
wrote:

Added Git tutorial, removed Subversion tutorial and updated multiple
references from Subversion to Git and from SourceForge to Kaldi's own web
site or GitHub as appropriate.

Please comment on particular issues in-line on GitHub page as the change

list is large.

You can view, comment on, or merge this pull request online at:

#50
Commit Summary

  • Documentation changes: added Git tutorial, removed Subversion
    tutorial and updated multiple references from Subversion to Git and from
    SourceForge to Kaldi's own web site or GitHub as appropriate.

File Changes

Patch Links:


Reply to this email directly or view it on GitHub
#50.

…r is not maintained; version control paragraph in tutorial_code.dox rewritten.
@jtrmal
Copy link
Contributor

jtrmal commented Aug 4, 2015

BTW, one more comment (of low importance, probably shouldn't be an an
obstacle in merging this PR): I think we can say nicer things about the
windows built now.
I wouldn't claim it's actively supported but the sentence there (woefully
old, or something like that) is too grim :)
y.

On Tue, Aug 4, 2015 at 12:06 AM, Daniel Povey notifications@github.com
wrote:

Thanks.. couple of points;
regarding dependencies, we still depend on svn being installed to install
some other stuff.

Regarding this, "
We still intend to maintain a+ read-only Subversion mirror of the GitHub
parent, located at SourceForge and mirrored+ by us."
this is no longer the plan, because Sourceforge lost our latest svn commits
and if we attempt to keep the SF up to date, existing SF users would likely
get corrupted copies of Kaldi.
There is a todo in your commit:
+\todo This paragraph is full of lies!
it would be good if you can find time to fix this before we merge the pull
request.
Dan

On Mon, Aug 3, 2015 at 8:37 PM, Kirill Katsnelson <
notifications@github.com>
wrote:

Added Git tutorial, removed Subversion tutorial and updated multiple
references from Subversion to Git and from SourceForge to Kaldi's own web
site or GitHub as appropriate.

Please comment on particular issues in-line on GitHub page as the change

list is large.

You can view, comment on, or merge this pull request online at:

#50
Commit Summary

  • Documentation changes: added Git tutorial, removed Subversion
    tutorial and updated multiple references from Subversion to Git and from
    SourceForge to Kaldi's own web site or GitHub as appropriate.

File Changes

Patch Links:


Reply to this email directly or view it on GitHub
#50.


Reply to this email directly or view it on GitHub
#50 (comment).

@kkm000
Copy link
Contributor Author

kkm000 commented Aug 4, 2015

@jtrmal:

probably shouldn't be an an obstacle in merging this PR

I think it should :-) As a sidekick thought, a PR might well represent a completed unit of work, since it is merged into the master branch as a functional feature. It makes perfect sense to discuss the changes and iteratively modify the change until it is logically complete. Git much better suited for this than svn, as it is much easier with it to have multiple branches and multiple PRs going at a time. So...

we can say nicer things about the windows built now.

...I'll change this later today, do not merge.

@kkm000
Copy link
Contributor Author

kkm000 commented Aug 4, 2015

@danpovey, @jtrmal, out of curiosity, are you getting a notification from GitHub when I push a commit to the existing PR? In the tutorial, I assumed not.

@jtrmal
Copy link
Contributor

jtrmal commented Aug 4, 2015

No, I didn't get informed about the last commit.
y.

On Tue, Aug 4, 2015 at 2:57 PM, Kirill Katsnelson notifications@github.com
wrote:

@danpovey https://github.com/danpovey, @jtrmal
https://github.com/jtrmal, out of curiosity, are you getting a
notification from GitHub when I push a commit to the existing PR? In the
tutorial, I assumed not.


Reply to this email directly or view it on GitHub
#50 (comment).

@kkm000
Copy link
Contributor Author

kkm000 commented Aug 4, 2015

Aha, and Dan was, although that was an out-of-thread e-mail message with a new subject! NB the commit message did literally contain @danpovey, and this one did not mention you. Interesting. Looks like GitHub would not notify the owner of the target repo of a push to the existing PR.

I have a line mentioning to respond with something like "Done" in the thread--looks like it makes total sense. Thanks for checking!

@jtrmal
Copy link
Contributor

jtrmal commented Aug 4, 2015

OK, I guess it's ready to merge?

@danpovey
Copy link
Contributor

danpovey commented Aug 4, 2015

I guess.
Won't hurt to merge it in any case. After you merge, I'll compile.
Dan

On Tue, Aug 4, 2015 at 12:26 PM, jtrmal notifications@github.com wrote:

OK, I guess it's ready to merge?


Reply to this email directly or view it on GitHub
#50 (comment).

jtrmal added a commit that referenced this pull request Aug 4, 2015
Documentation changes related to Git migration
@jtrmal jtrmal merged commit ec358b9 into kaldi-asr:master Aug 4, 2015
@danpovey
Copy link
Contributor

danpovey commented Aug 4, 2015

Thanks. Compiling now. Yenda, in a few minutes the docs will appear
online, could you please make sure there are no gross formatting errors?
Dan

On Tue, Aug 4, 2015 at 12:29 PM, jtrmal notifications@github.com wrote:

Merged #50 #50.


Reply to this email directly or view it on GitHub
#50 (comment).

@jtrmal
Copy link
Contributor

jtrmal commented Aug 4, 2015

yep, will do. y.

On Tue, Aug 4, 2015 at 3:32 PM, Daniel Povey notifications@github.com
wrote:

Thanks. Compiling now. Yenda, in a few minutes the docs will appear
online, could you please make sure there are no gross formatting errors?
Dan

On Tue, Aug 4, 2015 at 12:29 PM, jtrmal notifications@github.com wrote:

Merged #50 #50.


Reply to this email directly or view it on GitHub
#50 (comment).


Reply to this email directly or view it on GitHub
#50 (comment).

@jtrmal
Copy link
Contributor

jtrmal commented Aug 4, 2015

everything seems fine.
y.

On Tue, Aug 4, 2015 at 3:33 PM, Jan Trmal jtrmal@gmail.com wrote:

yep, will do. y.

On Tue, Aug 4, 2015 at 3:32 PM, Daniel Povey notifications@github.com
wrote:

Thanks. Compiling now. Yenda, in a few minutes the docs will appear
online, could you please make sure there are no gross formatting errors?
Dan

On Tue, Aug 4, 2015 at 12:29 PM, jtrmal notifications@github.com wrote:

Merged #50 #50.


Reply to this email directly or view it on GitHub
#50 (comment).


Reply to this email directly or view it on GitHub
#50 (comment).

@kkm000
Copy link
Contributor Author

kkm000 commented Aug 4, 2015

@jtrmal Thanks for checking.

@kkm000 kkm000 deleted the gittutorial branch August 4, 2015 22:50
hainan-xv pushed a commit to hainan-xv/kaldi that referenced this pull request Jul 24, 2018
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