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

Remove opensuse/tumbleweed #1987

Merged
merged 1 commit into from
Oct 27, 2021

Conversation

kevinbackhouse
Copy link
Collaborator

@kevinbackhouse kevinbackhouse commented Oct 24, 2021

This workflow is failing because the opensuse/tumbleweed docker image is currently broken. It's a known issue that isn't getting fixed quickly, so I think it's better to just drop tumbleweed for now.

@codecov
Copy link

codecov bot commented Oct 24, 2021

Codecov Report

Merging #1987 (83da236) into main (ab90d43) will not change coverage.
The diff coverage is n/a.

❗ Current head 83da236 differs from pull request most recent head befb890. Consider uploading reports for the commit befb890 to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1987   +/-   ##
=======================================
  Coverage   61.29%   61.29%           
=======================================
  Files          96       96           
  Lines       19110    19110           
  Branches     9760     9760           
=======================================
  Hits        11713    11713           
  Misses       5076     5076           
  Partials     2321     2321           

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 ab90d43...befb890. Read the comment docs.

Copy link
Collaborator

@piponazo piponazo left a comment

Choose a reason for hiding this comment

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

Fine for me to drop opensuse temporarily.

Nonetheless, I would add an issue in our backlog to try to add it back in the future. These issues are more frequent than one might suspect/desire ... eventually we could end up dropping checks for many distributions without remembering to add them back.

@kmilos
Copy link
Collaborator

kmilos commented Oct 26, 2021

Maybe it's just me, but I really don't see the benefit (to exiv2) in testing so many different Linux distributions anyway - it's not like exiv2 has a huge dependency chain to tests... I'd leave that for the distro packagers. The priority IMHO should be on platform/OS+toolchain, not every distro out there, however "major" it is...

@piponazo
Copy link
Collaborator

Maybe it's just me, but I really don't see the benefit (to exiv2) in testing so many different Linux distributions anyway - it's not like exiv2 has a huge dependency chain to tests... I'd leave that for the distro packagers. The priority IMHO should be on platform/OS+toolchain, not every distro out there, however "major" it is...

Maybe you are right and at this point it does not make sense anymore. In the past we added support to several distributions trying to solve issues reported in some of them. In other cases we just added more linux distributions because it was easy to do it after all the infrastructure added by @D4N. For me it is still interesting to see if the newer toolchains included by some distributions might trigger some warnings/errors that are not seen in the default ubuntu images we use to do most of our checks.

As we can see in this PR, at this point to add/remove support for different distributions we just need to add few keywords here & there. IMHO, I think it is not very painful to keep support for most of them. Also, as we saw in this case, if one specific distro is giving us head aches, we can drop it temporarily or permanently.

@kevinbackhouse
Copy link
Collaborator Author

Thanks! I created #1989 to remind myself to revert this when the issue with the docker image is fixed.

@kevinbackhouse kevinbackhouse merged commit 5dca23b into Exiv2:main Oct 27, 2021
@kevinbackhouse kevinbackhouse added this to the v1.00 milestone Dec 21, 2021
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.

3 participants