Skip to content

Conversation

@HorlogeSkynet
Copy link
Member

@HorlogeSkynet HorlogeSkynet commented Apr 21, 2020

Hey over here 👋

So, there is a bug an unexpected behavior with distro in very "lite" environments, when for example even /etc/os-release is not available.

One-liner to reproduce : docker run archlinux:latest /bin/bash -c 'pacman --noconfirm -Sy python-distro && python3 -m distro'

It actually appears that we should be dealing with /usr/lib/os-release in such cases (reference).

This patch implements the expected behavior, and adds a very specific test case to verify it.

Bye 🙇

@HorlogeSkynet
Copy link
Member Author

HorlogeSkynet commented Apr 26, 2020

Note : the patch seems back-portable to the python2.6-support branch.

UPDATE 2020-06-02 : Not anymore, since it relies on #247 (not back-ported) too now.

@HorlogeSkynet
Copy link
Member Author

Rebased against new master, fixing conflicts to include changes brought by #247.

(cc @nir0s)

@HorlogeSkynet
Copy link
Member Author

Up @nir0s please 🙄

@HorlogeSkynet
Copy link
Member Author

Up... again @nir0s (cc @hartwork maybe ?) 😕

@HorlogeSkynet
Copy link
Member Author

One more "up" for 2020.

@hartwork
Copy link
Contributor

hartwork commented Dec 31, 2020

Up... again @nir0s (cc @hartwork maybe ?) confused

I don't have capacity for proper review here atm, I'll leave this to @nir0s for now.
I left a thumbs up on the general idea on top though, from a quick look it looks like a good move.

@nir0s
Copy link
Collaborator

nir0s commented Dec 31, 2020

Fellas, I truly apologize, but I indeed hardly have any capacity to deal with issues nowadays.

I'd appreciate adding collaborators. Any volunteers?

@HorlogeSkynet
Copy link
Member Author

HorlogeSkynet commented Dec 31, 2020

Well, thanks for your answer @nir0s. I can't go full-time on it of course, but as I now maintain a project depending completely on yours, I'm indirectly interesting in distro's "health condition".
So I'm okay for sharing work with other(s), if it's really needed.

Bye, take care.

@hartwork
Copy link
Contributor

Fellas, I truly apologize, but I indeed hardly have any capacity to deal with issues nowadays.

I think it's good and fair to be realistic and open about it, thanks for that. And thanks for your work on distro in general 🙏 At the same time I think it's a bit sad also e.g. because of the 9000+ projects dependening on distro today (on GitHub alone) including important players like SaltStack, Let's Encrypt Certbot, docker-compose and package manager conan. So the 154 stars up here are only the tip of the iceberg.

If I may ask, are your capacity constraints temporary or expected long-term? If long-term, maybe it makes sense to turn this project into a new GitHub org then. It's rather easy to do technically — create the org, then do a repo transfer (which moves all issues and PRs just like that, then add people) — and it may communicate better that the project has become the effort of a new team if you're saying goodbye long-term. At least one other trusted person will also need PyPI access for a full transition. I hope I'm being respectful here, I didn't intend otherwise. What do you think?

I'd appreciate adding collaborators. Any volunteers?

Speaking for myself, I will not be able to shoulder much load here but if there are other active contributors, I may be able to join and help a bit myself too (after my current sprint on https://github.com/git-big-picture/git-big-picture has cooled down more).

Copy link
Contributor

@hartwork hartwork left a comment

Choose a reason for hiding this comment

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

I had a closer look now. Given what the spec says about /usr/lib/os-release I think merging some version of this pull request will be important to do justice to the specification. So thanks to @HorlogeSkynet for working on this matter! 🙏
Below are my findings. Some of them are picky… but with good intentions. I'm happy to learn I misunderstood or overlooked something. Best, Sebastian

hartwork
hartwork previously approved these changes Jan 10, 2021
Copy link
Contributor

@hartwork hartwork left a comment

Choose a reason for hiding this comment

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

@HorlogeSkynet looks good to me now, thanks for your contribution and your patience! 🙏

@HorlogeSkynet
Copy link
Member Author

@HorlogeSkynet looks good to me now, thanks for your contribution and your patience! 🙏

You're very welcome, sorry for this little leftover that didn't catch my attention sooner.
Bye 🙇

@hartwork
Copy link
Contributor

hartwork commented Jan 16, 2021

@nir0s this PR is ready and waiting for your approval by now. Do you have a minute?

@hartwork
Copy link
Contributor

@nir0s any news?

@hartwork
Copy link
Contributor

hartwork commented Mar 6, 2021

@nir0s how can we move forward?

@HorlogeSkynet
Copy link
Member Author

So sad only pull requests "brandishing" the fork totem are able to get things move forward... 🙁

@hartwork
Copy link
Contributor

@nir0s this PR is ready and waiting for your approval by now. Do you have a minute?

@nir0s please please

Copy link
Contributor

@sethmlarson sethmlarson left a comment

Choose a reason for hiding this comment

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

Overall this looks like a good improvement. Had some comments for you:

sethmlarson
sethmlarson previously approved these changes Jul 1, 2021
Copy link
Contributor

@sethmlarson sethmlarson left a comment

Choose a reason for hiding this comment

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

Going to approve since the reference comment is optional.

@nir0s
Copy link
Collaborator

nir0s commented Jul 1, 2021

There are conflicts now, though. @HorlogeSkynet.

@HorlogeSkynet
Copy link
Member Author

There are conflicts now, though. @HorlogeSkynet.

🙄 I'll try to rebase soon.

@hartwork
Copy link
Contributor

hartwork commented Jul 1, 2021

🙄 I'll try to rebase soon.

@HorlogeSkynet let me know if you need help

@HorlogeSkynet HorlogeSkynet dismissed stale reviews from sethmlarson and hartwork via 1f61af4 July 1, 2021 18:32
@HorlogeSkynet
Copy link
Member Author

@nir0s I went for a manual merge, all good now ✅

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.

4 participants