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

better warning for users when the buildconfig is of type binary #9678

Merged
merged 2 commits into from
Jul 2, 2016
Merged

better warning for users when the buildconfig is of type binary #9678

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
52bbdf0
better warning for users when the buildconfig is of type binary
bparees Jul 1, 2016
d283cd2
bump(github.com/openshift/source-to-image): ebd41a8b24bd95716b90a05fe…
bparees Jul 2, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 19 additions & 19 deletions Godeps/Godeps.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions Godeps/_workspace/src/github.com/openshift/source-to-image/pkg/scm/git/git.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions Godeps/_workspace/src/github.com/openshift/source-to-image/pkg/scm/git/git_test.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 11 additions & 4 deletions pkg/generate/app/cmd/describe.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,11 +171,18 @@ func describeBuildPipelineWithImage(out io.Writer, ref app.ComponentReference, p
fmt.Fprintf(out, " * The resulting image will be pushed to %s %q\n", to.Kind, to.Name)
}
}
if len(trackedImage) > 0 {
if noSource {
fmt.Fprintf(out, " * Use 'start-build --from-dir=DIR' to trigger a new build\n")
} else {

if noSource {
// if we have no source, the user must always provide the source from the local dir(binary build)
fmt.Fprintf(out, " * Use 'start-build --from-dir=DIR|--from-repo=DIR|--from-file=FILE' to trigger a new build\n")
Copy link
Contributor

Choose a reason for hiding this comment

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

how bout start-build --from-dir=DIR | start-build --from-repo=DIR | start-build --from-file=FILE ?

but don't feel super strong about it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

too lengthy, it'd wrap the screen. (the warning already does, but having two things in a row wrap would be really ugly)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oops. actually this is already wrapping the screen. but i'd still prefer not to make it longer.

Copy link
Contributor

Choose a reason for hiding this comment

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

gotcha ... i'm good

Copy link

Choose a reason for hiding this comment

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

fmt.Fprintf(out, " * WARNING: a binary build was created, you must specify one of --from-dir|--from-file|--from-repo when starting builds\n")
} else {
if len(trackedImage) > 0 {
// if we have a trackedImage/ICT and we have source, the build will be triggered automatically.
fmt.Fprintf(out, " * Every time %q changes a new build will be triggered\n", trackedImage)
} else {
// if we have source (but not a tracked image), the user must manually trigger a build.
fmt.Fprintf(out, " * Use 'start-build to trigger a new build\n")
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion pkg/generate/app/sourcelookup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,8 @@ func (r *SourceRepository) RemoteURL() (*url.URL, bool, error) {
if len(ref) > 0 {
remote = fmt.Sprintf("%s#%s", remote, ref)
}
if r.remoteURL, err = url.Parse(remote); err != nil {

if r.remoteURL, err = git.ParseRepository(remote); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

there you go ... i had thought either @csrwng or I had made this change in new-app, but nope ... good catch

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it was made in some places but not all, which led to some interesting behavior.

Copy link
Contributor

Choose a reason for hiding this comment

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

wonder if there were some additions after we made the change ... no matter though at this point

return nil, false, err
}
default:
Expand Down
71 changes: 0 additions & 71 deletions pkg/generate/app/sourceref.go
View file
Open in desktop

This file was deleted.

40 changes: 0 additions & 40 deletions pkg/generate/app/sourceref_test.go
View file
Open in desktop

This file was deleted.