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

[MM-16285] update circleci #69

Merged
merged 4 commits into from
Jun 25, 2019
Merged

Conversation

mickmister
Copy link
Contributor

@mickmister mickmister commented Jun 21, 2019

Updating circleci config to reflect the configuration used by up-to-date plugins. It looks like there is some existing configuration made by @manland that will be removed by this change.

https://mattermost.atlassian.net/browse/MM-16285

@mickmister mickmister changed the title update circleci [MM-16285] update circleci Jun 21, 2019
@crspeller
Copy link
Member

@mickmister We can perserve the coverage job. Hanzei added the coverage job to the orb: https://github.com/mattermost/circleci-orbs/blob/master/plugin-ci/orb.yml#L57 so you can just add it back here.

@mickmister
Copy link
Contributor Author

@crspeller Should I add it back just as it was in this repo, before this PR? Or a different config that matches what orb.yml is expecting?

@crspeller
Copy link
Member

^ Answered offline.

@manland
Copy link
Contributor

manland commented Jun 21, 2019

Thanks really cool job! My coverage job never worked 😁

And thanks to @hanzei for adding coverage in orb!

@crspeller we need #70 to repair test but my pr don't launch ci! Do you know why? And how to fix it?

@crspeller
Copy link
Member

@manland The setting to build forks was disabled in CircleCI. Fixed it.

@mickmister
Copy link
Contributor Author

Coverage job added

@hanzei hanzei added the 2: Dev Review Requires review by a core committer label Jun 22, 2019
Copy link
Collaborator

@hanzei hanzei left a comment

Choose a reason for hiding this comment

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

Nit: Do you need both a plugin-ci/coverage and a plugin-ci/test job in your workflow?

@mickmister
Copy link
Contributor Author

@hanzei I'm not sure I understand, I'm not too familiar with what each job is doing. Does the plugin-ci/test job create coverage results as it finishes? Also, does the order of the jobs matter?

How do you suggest configuring the workflow?

@hanzei
Copy link
Collaborator

hanzei commented Jun 23, 2019

Does the plugin-ci/test job create coverage results as it finishes?

No. But plugin-ci/coverage create a coverage result and uploads it. But because plugin-ci/coverage also tests the code, it supersedes plugin-ci/test.

Also, does the order of the jobs matter?

I would suggest you read through https://circleci.com/docs/2.0/workflows/. The docuentation explains this topic better then I could.

How do you suggest configuring the workflow?

1/5:

version: 2.1

orbs:
  plugin-ci: mattermost/plugin-ci@volatile

workflows:
  version: 2
  ci:
    jobs:
      - plugin-ci/lint:
          filters:
            tags:
              only: /^v.*/
      - plugin-ci/coverage:
          filters:
            tags:
              only: /^v.*/
      - plugin-ci/build:
          filters:
            tags:
              only: /^v.*/
      - plugin-ci/deploy-ci:
          filters:
            branches:
              only: master
          context: plugin-ci
          requires:
            - plugin-ci/lint
            - plugin-ci/coverage
            - plugin-ci/build
      - plugin-ci/deploy-release:
          filters:
            tags:
              only: /^v.*/
            branches:
              ignore: /.*/
          context: plugin-ci
          requires:
            - plugin-ci/lint
            - plugin-ci/coverage
            - plugin-ci/build
      - plugin-ci/deploy-release-github:
          filters:
            tags:
              only: /^v.*/
            branches:
              ignore: /.*/
          context: matterbuild-github-token
          requires:
            - plugin-ci/lint
            - plugin-ci/coverage
            - plugin-ci/build

@mickmister
Copy link
Contributor Author

Thank you for the explanation and docs link, @hanzei. I agree with the config, and have updated the PR.

@hanzei
Copy link
Collaborator

hanzei commented Jun 24, 2019

@crspeller Would you please check if codecov is enabled for this repo and enable it if not?

@manland
Copy link
Contributor

manland commented Jun 24, 2019

Hi thanks for this PR!

I think there is a little problem when replacing test by coverage : coverage don't fail if a test fail!

Sélection_251

https://circleci.com/gh/mattermost/mattermost-plugin-gitlab/291?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link

@hanzei
Copy link
Collaborator

hanzei commented Jun 24, 2019

On issue with this setup is that the caching doesn't work. The orb expects the go.mod file to be at /go.mod. But in this project it's at /server/go.mod.
I see
Screenshot from 2019-06-24 07-45-29

Possible options:

  1. Ignore this problem
  2. Move go.mod into the root directory
  3. Modify the orb to make this customizable

Thoughts on this?

@hanzei
Copy link
Collaborator

hanzei commented Jun 24, 2019

I think there is a little problem when replacing test by coverage : coverage don't fail if a test fail!

Good catch @manland, this should not be happening. I opened a ticket for this.

@crspeller
Copy link
Member

@hanzei How do I check if codecov is enabled?

@hanzei
Copy link
Collaborator

hanzei commented Jun 24, 2019

@crspeller Sending you a DM

@mickmister mickmister added 4: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a core committer labels Jun 25, 2019
@mickmister mickmister merged commit 07ab3fc into master Jun 25, 2019
@hanzei hanzei deleted the issue/MM-16285-update-circleci branch June 26, 2019 05:01
This was referenced Jul 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4: Reviews Complete All reviewers have approved the pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants