-
Notifications
You must be signed in to change notification settings - Fork 29
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
CONFIG: Migrate from CirclieCI to Codefresh [DEVSECOPS-114] #234
CONFIG: Migrate from CirclieCI to Codefresh [DEVSECOPS-114] #234
Conversation
codefresh.yml
Outdated
test: | ||
title: "Run Python tests" | ||
type: "freestyle" # Run any command | ||
image: "python:${{PYTHON_VERSION}}" # The image in which command will be executed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd just hard-code the Python version here rather than loading it from a project variable.
The reason for this is that if we want to upgrade to a different Python version, it's likely that there will be several other places in the repo that need to be updated (e.g. setup.py
, README.md
, docker-compose.yml
), so we could update this at the same time with a simple find-and-replace as opposed to an additional manual step.
codefresh.yml
Outdated
- apt-get update && apt-get install sudo -y | ||
|
||
# Install surround | ||
- sudo python setup.py install |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- sudo python setup.py install | |
- python3 setup.py install |
It's generally a good idea to run python2
or python3
as opposed to python
so that it's explicit which version of Python you're using.
(It's a moot point in this instance since we're using a python
Docker image, but it's easier to just do it everywhere for consistency).
codefresh.yml
Outdated
# Run tests | ||
- sudo python setup.py test | ||
|
||
# Run pylint tests | ||
- pylint setup.py | ||
- find surround/ -iname "*.py" | xargs pylint | ||
- find examples/ -iname "*.py" | xargs pylint | ||
|
||
# Run examples | ||
- ls examples/ | xargs -n 1 -I '{}' python examples/'{}'/main.py |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These three things would be good candidates for parallel steps and would speed things up considerably.
Not sure how long the build/tests actually take though, so could be outside the scope of this PR.
codefresh.yml
Outdated
when: | ||
branch: | ||
only: | ||
- master |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did the old CircleCI pipeline deploy to Pypi on every commit to master
, or only tags?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Like this - every time. The Pypi upload will fail if the version already exists.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can safely remove master
from the when
clause, because it won't work anyway if there's no tag.
Reason: git tag -l --points-at HEAD
returns nothing, which makes VERSION_TAG
empty, which means python3 setup.py
will raise an exception.
Fixed Pypi config bug and addressed PR feedback. |
@ucokzeko Can you review this when you have a chance? |
codefresh.yml
Outdated
approval: | ||
title: "Pending Approval" | ||
type: pending-approval | ||
description: "Please approve the release to Pypi." | ||
timeout: | ||
duration: 2 | ||
finalState: approved | ||
when: | ||
branch: | ||
only: | ||
- master | ||
- /([0-9]+)\.([0-9]+)\.([0-9]+)?$/ | ||
stage: "release" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd be inclined to remove this. Tagging the repo with a version number should be enough of a deliberate action in this case.
codefresh.yml
Outdated
when: | ||
branch: | ||
only: | ||
- master |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can safely remove master
from the when
clause, because it won't work anyway if there's no tag.
Reason: git tag -l --points-at HEAD
returns nothing, which makes VERSION_TAG
empty, which means python3 setup.py
will raise an exception.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Why did I make these changes?
Future tags of this repo should remove the "v" prefix.
To do
What are the changes?
Checklist
How has this been tested?
Inline pipeline using Codefresh website