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

fix(plugin): miscellaneous #6

Closed
wants to merge 23 commits into from

Conversation

DanielHabenicht
Copy link
Contributor

@DanielHabenicht DanielHabenicht commented Apr 16, 2019

closes: #4 #5

@DanielHabenicht
Copy link
Contributor Author

@cmurczek as Docker is a requirement for running tests now, you may have to switch to the Azure DevOps Hosted Ubuntu Agents as they are the only ones that have Docker installed.

@DanielHabenicht
Copy link
Contributor Author

DanielHabenicht commented Apr 17, 2019

  • Publish step is returning an Array instead of an Object
The publish plugins must return an Object.

The publish function of the semantic-release-docker-test returned [ { completeImageName: [ 'danielhabenicht/phonebook:1.0.3', 'danielhabenicht/phonebook:latest' ] }, { completeImageName: [ 'danielhabenicht/builder:1.0.3', 'danielhabenicht/builder:latest' ] } ] instead.

We recommend to report the issue to the semantic-release-docker-test authors, providing the following informations:

    * The semantic-release version: 15.12.3
    * The semantic-release logs from your CI job
    * The value returned by the plugin: [ { completeImageName: [ 'danielhabenicht/phonebook:1.0.3', 'danielhabenicht/phonebook:latest' ] }, { completeImageName: [ 'danielhabenicht/builder:1.0.3', 'danielhabenicht/builder:latest' ] } ]    * A link to the semantic-release plugin developer guide: https://github.com/semantic-release/semantic-release/blob/master/docs/developer-guide/plugin.md

@cmurczek-it
Copy link
Member

@DanielHabenicht thanks for the effort! A couple of things though to consider when submitting a PR:

  • instead of one PR for all changes create multiple smaller PRs. This allows for accepting those parts that are ready and continuing work on other, resp. rejecting changes that are no longer needed or unwanted
  • follow the commit message format described by conventional changelog/angular convention to allow semantic release doing its work properly (breaking changes - > bump the first part, feat -> bumps the second part, fix, perf -> bumps the third part of the versionnumber)

Please refactor this PR so I can accept the individual parts without your changes regarding available docker images for testing. I already solved that by stubbing the dependency during tests altogether.

@DanielHabenicht
Copy link
Contributor Author

DanielHabenicht commented Apr 26, 2019

Hi, thanks for maintaining this repo.
I have refactored the code so you can easily bump the version number according to the issues.

I will however not refactor the tests.
The problem there was simply, that the AzureDevOps Pipeline Agents are slower than my local machine thus running into the specified timeout.
The tests are written as End-2-End tests in order to test the plugin and its interactions with the docker engine. Just testing the output of the Plugin would be, in my mind, to narrow.
They do not need any preconfigured or downloaded docker images, as they are building and deleting there own.

Closing in favor for #7 and #8.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

IMPROVE: Add ability to push multiple docker images
2 participants