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 docker image naming issue for CI #324

Merged
merged 1 commit into from
Jul 6, 2016
Merged

Conversation

MorrisJobke
Copy link
Member

  • wrongly named PHP7 - it is PHP 5.6
  • moved integration tests at the very end

cc @rullzer @LukasReschke @Mar1u5

@MorrisJobke MorrisJobke added the 3. to review Waiting for reviews label Jul 6, 2016
@MorrisJobke MorrisJobke added this to the Nextcloud Next milestone Jul 6, 2016
@mention-bot
Copy link

@MorrisJobke, thanks for your PR! By analyzing the annotation information on this pull request, we identified @LukasReschke to be a potential reviewer

@nickvergessen
Copy link
Member

Error: image morrisjobke/nextcloud-ci-php5.6 not found

Also can we move js tests after unit tests as well?

@icewind1991
Copy link
Member

the docker image is still called -php7 from what I can tell, wouldn't that need to be renamed to

@MorrisJobke
Copy link
Member Author

the docker image is still called -php7 from what I can tell, wouldn't that need to be renamed to

I leave it as php7 and simply created a copy (to not break all open PRs). I also created the same images in the nextcloudci account on docker hub.

Let's see if this works now.

* wrongly named PHP7 - it is PHP 5.6
* moved integration tests at the very end
@MorrisJobke
Copy link
Member Author

Also can we move js tests after unit tests as well?

I would like to keep it there, as it only took 30 seconds to complete. The next version of drone will most likely support parallel execution of those steps. It will also have a proper UI to differentiate between the jobs.

jsunit:
image: morrisjobke/nextcloud-ci-jsunit:1.0.2
image: nextcloudci/jsunit:1.0.6
Copy link
Member

Choose a reason for hiding this comment

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

What is the reason to use explicit tags?

Copy link
Member Author

Choose a reason for hiding this comment

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

To exactly know on which version it was run on. In this case I just "updated" to the latest version because I know it was build successfully. Otherwise the CI could have a "latest" version locally and doesn't fetch the actual "latest" version from docker hub.

Copy link
Member Author

Choose a reason for hiding this comment

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

Better safe than sorry and debugging hours for just noticing, that different images are in use.

Copy link
Member

Choose a reason for hiding this comment

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

fair enough

@nickvergessen
Copy link
Member

👍

@rullzer
Copy link
Member

rullzer commented Jul 6, 2016

LGTM

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

Successfully merging this pull request may close these issues.

None yet

5 participants