Move workflow stuff out of pkg #315
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mmlb
force-pushed
the
move-workflow-stuff-out-of-pkg
branch
3 times, most recently
from
a32aa8a to
e0d3a62
Compare
Codecov Report
@@ Coverage Diff @@
## master #315 +/- ##
=========================================
Coverage ? 23.79%
=========================================
Files ? 14
Lines ? 1244
Branches ? 0
=========================================
Hits ? 296
Misses ? 928
Partials ? 20
Continue to review full report at Codecov.
|
mmlb
force-pushed
the
move-workflow-stuff-out-of-pkg
branch
from
e0d3a62 to
6ecf644
Compare
Closed
|
Is it fixing this as well #317 ? |
mmlb
added
the
breaking-change
detiber
previously approved these changes
Oct 5, 2020
Thanks for the review.
soon it shall be no more.
👍 |
|
I'll leave this up for @gianarb til tomorrow. |
gianarb
previously approved these changes
Oct 7, 2020
pkg is really just another name for utils and thats a bad pattern to have. We should have small concise packages over large expansive ones. Signed-off-by: Manuel Mendez <[email protected]>
Every caller of ParseYAML always calls ValidateTemplate right after. Why would we ever want to allow an invalid Template anyway? Signed-off-by: Manuel Mendez <[email protected]>
There aren't any other data formats now or in the forseeable future. Signed-off-by: Manuel Mendez <[email protected]>
The if check should have been:
```go
if err == nil && test.expectedError {
```
otherwise the `err != nil` short-circuits and `test.expectedError` is
never checked.
But there's no real reason to check the value of `err` since we can just
assert that it matches what we expected instead.
Signed-off-by: Manuel Mendez <[email protected]>
Makes no sense. Signed-off-by: Manuel Mendez <[email protected]>
This was changed when validation was added when the code was changed from fmt.Errorf -> errors.New, but errors.Errorf should have been used instead. Signed-off-by: Manuel Mendez <[email protected]>
This way we can group them by what they apply to. Signed-off-by: Manuel Mendez <[email protected]>
Signed-off-by: Manuel Mendez <[email protected]>
Signed-off-by: Manuel Mendez <[email protected]>
No need to have unique strings, and avoids needing another unique long string for templates if we want to continue with the old pattern. Signed-off-by: Manuel Mendez <[email protected]>
Makes no sense to add template here too, thats all the objects with name paramenter... Signed-off-by: Manuel Mendez <[email protected]>
Signed-off-by: Manuel Mendez <[email protected]>
golangci-lint was printing the following warning: WARN [runner/nolint] Found unknown linters in //nolint directives: staticcheck sa1019 we will do it later Signed-off-by: Manuel Mendez <[email protected]>
mmlb
force-pushed
the
move-workflow-stuff-out-of-pkg
branch
from
6ecf644 to
80db407
Compare
gianarb
approved these changes
Oct 7, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Move workflow types and funcs out of pkg/ and into its own package. Also cleaned up some of the API and added some more validation while I'm here.
Why is this needed
Helps address #300
How Has This Been Tested?
go testpasses.How are existing users impacted? What migration steps/scripts do we need?
Low impact. A bc-break is occurring here, but its in internalish code and no user is expected to be using this code. If they are
gofmt -w -r 'p.ParseYAML(b) -> p.Parse(b)' .and removing the call toValidateTemplateis all thats needed.Checklist:
I have: