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

Reorganize to docker action #17

Merged
merged 10 commits into from
Jan 27, 2022
Merged

Reorganize to docker action #17

merged 10 commits into from
Jan 27, 2022

Conversation

asvetlov
Copy link
Member

@asvetlov asvetlov commented Jan 27, 2022

No description provided.

@asvetlov asvetlov changed the title analyze dists Reorganize to docker action Jan 27, 2022
@codecov
Copy link

codecov bot commented Jan 27, 2022

Codecov Report

Merging #17 (55c978f) into master (ec71027) will increase coverage by 3.63%.
The diff coverage is 74.69%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #17      +/-   ##
==========================================
+ Coverage   70.17%   73.80%   +3.63%     
==========================================
  Files           1        1              
  Lines          57      126      +69     
  Branches       10       27      +17     
==========================================
+ Hits           40       93      +53     
- Misses         16       27      +11     
- Partials        1        6       +5     
Flag Coverage Δ
unit ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
get_releasenote.py 73.80% <74.69%> (+3.63%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 279eb98...55c978f. Read the comment docs.

@asvetlov asvetlov merged commit 5ef183f into master Jan 27, 2022
@asvetlov asvetlov deleted the analyze-dists branch January 27, 2022 21:31
INPUT_FIX_ISSUE_REGEX: ${{ inputs.fix_issue_regex }}
INPUT_FIX_ISSUE_REPL: ${{ inputs.fix_issue_repl }}
using: docker
image: Dockerfile
Copy link
Member

Choose a reason for hiding this comment

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

Note that with this approach, every job using the action will build the image. I've seen some actions optimizing this by publishing the built images to registries and pointing this there instead. Notably this one: https://github.com/fedora-python/tox-github-action/blob/9f7d1ff/action.yaml#L14.

Copy link
Member Author

Choose a reason for hiding this comment

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

Interesting.
Does Github cache the built docker image?
An explicit image registry complicates the release process. Not drastically but still extra steps.
The project was super-lightweight initially. Now it grows :(

Copy link
Member

Choose a reason for hiding this comment

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

Does Github cache the built docker image?

It does not. This is why I emphasized that this happens in every job.

An explicit image registry complicates the release process. [...] The project was super-lightweight initially. Now it grows :(

Yep, this is why I mentioned it. I still haven't built good automation for this but I've been wanting to use this approach for some of my actions like PyPI upload for a while now.

setup.cfg Show resolved Hide resolved
@@ -71,13 +70,26 @@ jobs:
uses: py-actions/py-dependency-install@v2
with:
path: requirements.txt
- name: Install Itself
Copy link
Member

Choose a reason for hiding this comment

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

FYI this is grammatically incorrect — saying that pip installs itself means that pip installs pip. Maybe it would be acceptable to say "Self-install" but even then, it would mean that the project installs itself (as in aiohttp installs aiohttp).
A better way could be to say "Install the tested project to the current environment in the editable mode"

Copy link
Member Author

Choose a reason for hiding this comment

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

Feel free to fix please

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