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.

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

Add Docker Publish Plugin #1510

Merged

Conversation

RichiCoder1
Copy link
Contributor

What Changed

Adds a Docker publish plugin.

Why

Simplifies publishing docker images with semver tags.

TODO:
I think most everything is done, possibly tests could be improved.

Copy link
Collaborator

@hipstersmoothie hipstersmoothie left a comment

Choose a reason for hiding this comment

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

This is looking awesome! Have you testing that any of this works?

Comment on lines 54 to 62
if (Array.isArray(options)) {
const errors = await Promise.all(
options.map((o) =>
validatePluginConfiguration(this.name, pluginOptions, o)
)
);

return errors.reduce((acc, item) => [...acc, ...item], []);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think this if statement is necessary. This would validate if a user provided an array of this plugin's option. I don't think this plugin allows for that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, got copy pasta'd. Tempted to add this later, but not necessary now.

}
});

auto.hooks.canary.tapPromise(this.name, async (version, postFix) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ooh nice! canary release is sweet

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seemed simple and a no-brainer to add :)

@RichiCoder1
Copy link
Contributor Author

This is looking awesome! Have you testing that any of this works?

Doing so now! Maybe because of Windows, a full yarn:test falls over on windows, so using filters & Circle CI to run em.

@codecov
Copy link

codecov bot commented Sep 4, 2020

Codecov Report

Merging #1510 into master will increase coverage by 0.11%.
The diff coverage is 87.34%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1510      +/-   ##
==========================================
+ Coverage   81.09%   81.20%   +0.11%     
==========================================
  Files          58       59       +1     
  Lines        4316     4395      +79     
  Branches      914      979      +65     
==========================================
+ Hits         3500     3569      +69     
- Misses        571      572       +1     
- Partials      245      254       +9     
Impacted Files Coverage Δ
plugins/docker/src/index.ts 87.34% <87.34%> (ø)

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 5fb3cc4...b69600f. Read the comment docs.

@RichiCoder1 RichiCoder1 marked this pull request as ready for review September 4, 2020 23:25
@RichiCoder1
Copy link
Contributor Author

Should be green and good now! Actually caught a logic bug with tests, so that's nice. Not sure how strict ya'll are about coverage, but at least worked to get green with codecov.

@hipstersmoothie
Copy link
Collaborator

@RichiCoder1 last few things:

can you add a reference to this plugin here and here

@RichiCoder1
Copy link
Contributor Author

Done! Do I need to regen docs or is that done as part of CI/Release?

@hipstersmoothie
Copy link
Collaborator

Done! Do I need to regen docs or is that done as part of CI/Release?

that's part of the release!

@hipstersmoothie hipstersmoothie merged commit 327241b into intuit:master Sep 8, 2020
@hipstersmoothie hipstersmoothie added the minor Increment the minor version when merged label Sep 8, 2020
@RichiCoder1
Copy link
Contributor Author

🥳

@RichiCoder1 RichiCoder1 deleted the feature/add_docker_plugin branch September 8, 2020 20:03
@adierkens
Copy link
Collaborator

🚀 PR was released in v9.53.0 🚀

@adierkens adierkens added the released This issue/pull request has been released. label Sep 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
minor Increment the minor version when merged released This issue/pull request has been released.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants