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

Refactor: context #463

Merged
merged 42 commits into from Dec 18, 2017

Conversation

Projects
None yet
3 participants
@caarlos0
Member

caarlos0 commented Dec 17, 2017

Refactor of the context package.

We now have an artifact package.

We now have an artifact type which can be filtered anywhere in the code, as well as filter compositions. With this set of features, we dont need specific fields for each type of artifact.

The mutex also is in the artifact package, and we have an Artifacts type, which contains both the slice and the mutex, and also have the Add, List and Filter methods.

I end up fixing some thing I found along the way and/or didn't work at all before:

  • docker ims from binary releases
  • multiple darwin amd64 builds on homebrew
  • Path now is always the full path (dist + folder + artifact) - before some pipes were using it differently and we have several filepath.Join(dist, artifact.Path) or similar around
  • the artifactory pipe was greatly simplified due to the previous changes
  • brew and artifactory pipes had some missing test cases which were added
  • the build pipe no longer needs to know anything about naming templates (it was a hidden dependency)
  • improved docs in several places

I also have placed several TODOs and opened a few issues to cover problems I've found.

This patch should allow us to add more types of artifacts easily, since all fields can be available.

I also included an Extra field which is a map[string]string, so some pipes can add additional info (right now only the build pipe is using it).

closes #455
closes #449

caarlos0 added some commits Dec 17, 2017

refactor: new artifact package
It contains the needed code to refactor the rest of the app
@codecov-io

This comment has been minimized.

codecov-io commented Dec 17, 2017

Codecov Report

Merging #463 into master will increase coverage by 0.49%.
The diff coverage is 93.31%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #463      +/-   ##
==========================================
+ Coverage   93.65%   94.14%   +0.49%     
==========================================
  Files          38       38              
  Lines        1513     1538      +25     
==========================================
+ Hits         1417     1448      +31     
+ Misses         57       52       -5     
+ Partials       39       38       -1
Impacted Files Coverage Δ
pipeline/git/git.go 100% <ø> (ø) ⬆️
goreleaserlib/goreleaser.go 94.52% <ø> (ø) ⬆️
internal/archiveformat/format.go 100% <ø> (ø) ⬆️
internal/buildtarget/buildtarget.go 100% <ø> (ø) ⬆️
pipeline/piper.go 100% <ø> (ø) ⬆️
pipeline/sign/sign.go 93.33% <100%> (+1.17%) ⬆️
internal/ext/ext.go 100% <100%> (ø) ⬆️
pipeline/release/body.go 100% <100%> (ø) ⬆️
context/context.go 100% <100%> (ø) ⬆️
checksum/checksum.go 100% <100%> (+13.33%) ⬆️
... and 13 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0d9b227...ca0b596. Read the comment docs.

@goreleaser goreleaser deleted a comment from GitCop Dec 17, 2017

@goreleaser goreleaser deleted a comment from GitCop Dec 17, 2017

@goreleaser goreleaser deleted a comment from GitCop Dec 17, 2017

@goreleaser goreleaser deleted a comment from GitCop Dec 17, 2017

@goreleaser goreleaser deleted a comment from GitCop Dec 17, 2017

@goreleaser goreleaser deleted a comment from GitCop Dec 17, 2017

@magiconair

This comment has been minimized.

Contributor

magiconair commented Dec 17, 2017

Maybe you can name the methods ByGOOS, ByGOARCH and ByGOARM since they refer to existing env var with these names.

@goreleaser goreleaser deleted a comment from GitCop Dec 17, 2017

@goreleaser goreleaser deleted a comment from GitCop Dec 17, 2017

@goreleaser goreleaser deleted a comment from GitCop Dec 17, 2017

@goreleaser goreleaser deleted a comment from GitCop Dec 17, 2017

caarlos0 added some commits Dec 17, 2017

@goreleaser goreleaser deleted a comment from GitCop Dec 17, 2017

@goreleaser goreleaser deleted a comment from GitCop Dec 17, 2017

caarlos0 added some commits Dec 17, 2017

@goreleaser goreleaser deleted a comment from GitCop Dec 17, 2017

@goreleaser goreleaser deleted a comment from GitCop Dec 17, 2017

@goreleaser goreleaser deleted a comment from GitCop Dec 17, 2017

@goreleaser goreleaser deleted a comment from GitCop Dec 17, 2017

@goreleaser goreleaser deleted a comment from GitCop Dec 18, 2017

@goreleaser goreleaser deleted a comment from GitCop Dec 18, 2017

caarlos0 added some commits Dec 18, 2017

@goreleaser goreleaser deleted a comment from GitCop Dec 18, 2017

@goreleaser goreleaser deleted a comment from GitCop Dec 18, 2017

@goreleaser goreleaser deleted a comment from GitCop Dec 18, 2017

@caarlos0 caarlos0 changed the title from WIP: refactor: context to Refactor: context Dec 18, 2017

@caarlos0

This comment has been minimized.

Member

caarlos0 commented Dec 18, 2017

cc/ @andygrunwald @jorinvo @elopio if you guys mind to take a look as well (sorry, the PR is really big)

@goreleaser goreleaser deleted a comment from GitCop Dec 18, 2017

caarlos0 added some commits Dec 18, 2017

@goreleaser goreleaser deleted a comment from GitCop Dec 18, 2017

caarlos0 added some commits Dec 18, 2017

@goreleaser goreleaser deleted a comment from GitCop Dec 18, 2017

@goreleaser goreleaser deleted a comment from GitCop Dec 18, 2017

@goreleaser goreleaser deleted a comment from GitCop Dec 18, 2017

@goreleaser goreleaser deleted a comment from GitCop Dec 18, 2017

@caarlos0

This comment has been minimized.

Member

caarlos0 commented Dec 18, 2017

OK, so let's try this.

@caarlos0 caarlos0 merged commit aaab7e1 into master Dec 18, 2017

4 of 6 checks passed

codecov/patch 93.31% of diff hit (target 93.65%)
Details
commit-message-check/gitcop Some commit messages are invalid
Details
WIP ready for review
Details
codecov/project 94.14% (+0.49%) compared to 0d9b227
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@caarlos0 caarlos0 deleted the ctx-new branch Dec 18, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment