-
Notifications
You must be signed in to change notification settings - Fork 244
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
Fix watch error message for git components #713
Conversation
@mik-dass You need to add descriptive messages to your PR's as well as Git commit messages. Describe the issue as well as why the fix is needed 👍 After you've updated that ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, lets merge once you add description to commit message
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 😄
A new error message is introduced for cases where 'watch' is initiated on a git component. Signed-off-by: mdas <mrinald7@gmail.com>
@cdrage Done. I also added a only after local components |
LGTM! 👍 |
checkError(err, "Unable to get source for %s component.", componentName) | ||
|
||
if sourceType != "binary" && sourcePath != "local" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❗️ ❗️ ❗️
I think you wanted:
sourceType != "binary" && sourceType != "local"
not:
sourceType != "binary" && sourcePath != "local"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah! Sorry my bad
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That happens, no worries.
What is disappointing is that no one bothered to run and test this manually, not one of 3 people
that reviewed and approved this and apparently not even you who wrote it :-(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry again. I tested locally but only some of the negative cases as it was a small fix. Will be careful in the future.
A new error message is introduced for cases where 'watch' is initiated on a git component. Signed-off-by: mdas <mrinald7@gmail.com>
fixes #665
A new error message is introduced for cases where 'watch' is initiated on a git component.