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

Liberating pipeline #365

Merged
merged 9 commits into from
Nov 11, 2020
Merged

Liberating pipeline #365

merged 9 commits into from
Nov 11, 2020

Conversation

wilko77
Copy link
Collaborator

@wilko77 wilko77 commented Oct 21, 2020

We had two problems with PR from external repos:

  1. they were not allowed to push to codecov.
  2. they were not allowed to publish to the artifact feed on Azure.

I changed the pipeline to:

  1. allow publishing to codecov to fail. (This should not block)
  2. we only publish to the feed from the master branch. (This makes sense as we only want to publish artifacts that have been reviewed.)

@codecov
Copy link

codecov bot commented Oct 21, 2020

Codecov Report

Merging #365 (9770607) into master (1f3902d) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #365   +/-   ##
=======================================
  Coverage   94.57%   94.57%           
=======================================
  Files          16       16           
  Lines         792      792           
=======================================
  Hits          749      749           
  Misses         43       43           

@wilko77
Copy link
Collaborator Author

wilko77 commented Nov 9, 2020

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Contributor

@joyceyuu joyceyuu left a comment

Choose a reason for hiding this comment

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

It seems Azure and Travis are happy with this so I will approve it

@@ -35,8 +35,9 @@ steps:
opSysFlag=$(echo ${{ parameters.operatingSystem }} | sed 's/[[:punct:]]//g' | tr '[:upper:]' '[:lower:]' | head -c30)
pyVFlag=$(echo python${{ parameters.pythonVersion }} | sed 's/[[:punct:]]//g' | tr '[:upper:]' '[:lower:]' | head -c30)
echo "codecov flags: $opSysFlag,$pyVFlag"
python -m codecov --token $(CODECOV_TOKEN) \
--file coverage.xml \
bash <(curl -s https://codecov.io/bash) \
Copy link
Contributor

Choose a reason for hiding this comment

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

no need for token anymore?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

the codecov tool reads the token from the environment variable. So no need to specify it here. -> Less clutter. :)

trigger:
branches:
include:
- '*'
Copy link
Contributor

Choose a reason for hiding this comment

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

why do you delete this section? The pipeline will no longer be triggered by PRs?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's actually the opposite. The pr: none bit prevented the pipeline to be triggered by a PR. However, we want every PR to trigger the tests. External PRs won't have access to the secrets, thus this is fine.
By removing all of this, we get the default behavior, which is run for every PR.

@wilko77 wilko77 merged commit 03ac3d0 into data61:master Nov 11, 2020
@wilko77 wilko77 deleted the liberating_pipeline branch November 11, 2020 01:04
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.

2 participants