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.

Sign up for GitHub

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

Jump to bottom

fix mispellings and add minor corrections #3258

Merged
merged 3 commits into from Aug 8, 2016
Merged

fix mispellings and add minor corrections #3258

merged 3 commits into from Aug 8, 2016

Conversation

Mistobaan
Copy link
Contributor

@Mistobaan Mistobaan commented Jul 10, 2016 •
edited

The main change of the PR is the change of interface from a single Reader(, isText) to a more (IMHO) go idiomatic ReadFromBuffer and ReadFromString

@tensorflow-jenkins
Copy link
Collaborator

Can one of the admins verify this patch?

@Mistobaan Mistobaan mentioned this pull request Jul 10, 2016
Closed
@alonsovidales
Copy link

Awesome 👍

vrv
vrv reviewed Jul 11, 2016
View reviewed changes
tensorflow/contrib/go/README.md
@@ -15,7 +15,7 @@ also to build, load and run Graphs.
## Installation

This package depends on the TensorFlow shared libraries, in order to compile
this libraries follow the [Installing fromsources](https://www.tensorflow.org/versions/r0.8/get_started/os_setup.html#installing-from-sources)
this libraries follow the [Installing fromsources](https://www.tensorflow.org/versions/r0.9/get_started/os_setup.html#installing-from-sources)
Copy link

Choose a reason for hiding this comment

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

https://www.tensorflow.org/get_started/os_setup.html#installing-from-sources

use that link and it'll redirect to the latest stable version

@vrv vrv self-assigned this Jul 11, 2016
@vrv vrv added the stat:awaiting response Status - Awaiting response from author label Jul 11, 2016
@vrv vrv changed the title fix mispless and add minor corrections fix mispellings and add minor corrections Jul 11, 2016
@vrv
Copy link

vrv commented Jul 15, 2016

(ping when ready to review)

@Mistobaan
Copy link
Contributor Author

Mistobaan commented Jul 19, 2016 •
edited

@vrv I think this PR is ready to be reviewed.

I tried to add some other things like the go bazel rules to build the package uzing bazel but I will wait for this PR to be merged and the branch to be rebased.

@aselle aselle added stat:awaiting tensorflower Status - Awaiting response from tensorflower and removed stat:awaiting response Status - Awaiting response from author labels Jul 19, 2016
@vrv
Copy link

vrv commented Jul 19, 2016

@tensorflow-jenkins test this please

I have no idea if we even have tests for this or what happens, but trying it anyway.

@Mistobaan
Copy link
Contributor Author

@vrv there are few tests, but I don't think they are linked to the CI system.

@Mistobaan Mistobaan mentioned this pull request Jul 20, 2016
Closed
3 tasks
@martinwicke
Copy link
Member

This didn't work, I guess.

@martinwicke martinwicke added stat:awaiting response Status - Awaiting response from author and removed stat:awaiting tensorflower Status - Awaiting response from tensorflower labels Jul 22, 2016
@Mistobaan
Copy link
Contributor Author

@martinwicke can you guys rebase origin/go on top of origin/master ? a bunch of PR are trying to catch up the go branch to the master one.

@martinwicke
Copy link
Member

I rebased the go branch on top of current master.

@Mistobaan
Copy link
Contributor Author

🎉 Thanks ! 🎉

@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm.

@Mistobaan
fix mispless and add minor corrections
aaadda5
@Mistobaan
fix documentation link
df7478b
@Mistobaan
remove non idiomatic NewGraphFromReader(reader, bool) and add NewGrap…
268d144 
…hFromBuffer and New Graph

NewGraphFromReader(reader, bool) and add NewGraphFromBuffer and NewGraphFromString.
The resulting code feels more elegant.  The user is responsible to manage the input.
@googlebot
Copy link

CLAs look good, thanks!

@Mistobaan
Copy link
Contributor Author

PR rebased too.

@teamdandelion
Copy link
Contributor

@vrv you've looked at this in the past. should we merge it?

@teamdandelion teamdandelion added awaiting review Pull request awaiting review stat:awaiting tensorflower Status - Awaiting response from tensorflower and removed stat:awaiting response Status - Awaiting response from author labels Aug 8, 2016
@teamdandelion teamdandelion self-assigned this Aug 8, 2016
@vrv
Copy link

vrv commented Aug 8, 2016

I dunno, there are no tests triggered for this branch, so merging might break the branch. @Mistobaan can fix it I guess if there are problems :)

@teamdandelion teamdandelion merged commit 4fac3fc into tensorflow:go Aug 8, 2016
@Mistobaan
Copy link
Contributor Author

👍

@Mistobaan Mistobaan deleted the go branch January 22, 2017 19:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@vrv vrv

@teamdandelion teamdandelion

Labels
awaiting review Pull request awaiting review cla: yes stat:awaiting tensorflower Status - Awaiting response from tensorflower
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@Mistobaan @tensorflow-jenkins @alonsovidales @vrv @martinwicke @googlebot @teamdandelion @aselle