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

Publish driftctl to Arch user repository #414

Merged
merged 2 commits into from
Apr 14, 2021
Merged

Publish driftctl to Arch user repository #414

merged 2 commits into from
Apr 14, 2021

Conversation

eliecharra
Copy link
Contributor

@eliecharra eliecharra commented Apr 8, 2021

Q A
πŸ› Bug fix? no
πŸš€ New feature? no
⚠ Deprecations? no
❌ BC Break no
πŸ”— Related issues #405
❓ Documentation no

Description

Add automatic publish to arch user repository.

Example pipeline can be found there : https://app.circleci.com/pipelines/github/cloudskiff/driftctl/1171/workflows/672dbdd4-d2cb-4ad1-aefe-4af1559288e4/jobs/1845

@eliecharra eliecharra added the kind/maintenance Refactoring or changes to the workspace label Apr 8, 2021
@eliecharra eliecharra added this to the v0.8.0 milestone Apr 8, 2021
@eliecharra eliecharra requested a review from a team April 8, 2021 15:34
@eliecharra eliecharra added this to Review in driftctl via automation Apr 8, 2021
@codecov
Copy link

codecov bot commented Apr 8, 2021

Codecov Report

Merging #414 (e26c331) into main (c1bddae) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #414   +/-   ##
=======================================
  Coverage   70.73%   70.73%           
=======================================
  Files         284      284           
  Lines        6396     6396           
=======================================
  Hits         4524     4524           
  Misses       1504     1504           
  Partials      368      368           

@eliecharra eliecharra marked this pull request as ready for review April 8, 2021 15:49
@eliecharra
Copy link
Contributor Author

@wbeuil @moadibfr It was quite complicated, attentive review required there πŸ™πŸ»

Copy link
Contributor

@wbeuil wbeuil left a comment

Choose a reason for hiding this comment

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

LGTM as for the script and the ci config, I tested it with a clone repo. Too bad there is not an easier way though ... Just two small comments about the badges

README.md Outdated
<br>
<a href="https://repology.org/project/driftctl/versions">
<img src="https://repology.org/badge/vertical-allrepos/driftctl.svg" alt="Packaging status">
</a>
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not huge fan of having something that big as a badge. We can use their tiny badges as per the docs of repology if we really want to display actual version for each package manager.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And what about something like this ?

Packaging status Packaging status

Copy link
Contributor

Choose a reason for hiding this comment

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

Didn't know we could do that in markdown ! I'm ok with that

README.md Outdated
@@ -6,7 +6,7 @@
<img src="https://circleci.com/gh/cloudskiff/driftctl.svg?style=shield"/>
<img src="https://goreportcard.com/badge/github.com/cloudskiff/driftctl"/>
<img src="https://img.shields.io/github/license/cloudskiff/driftctl">
<img src="https://img.shields.io/github/v/release/cloudskiff/driftctl">
<img src="https://img.shields.io/github/v/release/cloudskiff/driftctl?label=latest%20version">
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the change of label ? The word release is enough self-explanatory.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I think it was unclear with the new block label, will revert this with the usage of individual labels

@eliecharra eliecharra force-pushed the publish-to-aur branch 4 times, most recently from fbf7fe0 to d59e6b2 Compare April 13, 2021 15:34
@eliecharra eliecharra requested a review from wbeuil April 13, 2021 15:34
@eliecharra eliecharra merged commit c091d04 into main Apr 14, 2021
driftctl automation moved this from Review to Done Apr 14, 2021
@eliecharra eliecharra deleted the publish-to-aur branch April 14, 2021 12:19
@moadibfr moadibfr added this to Done in driftctl Apr 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/maintenance Refactoring or changes to the workspace
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants