Use safe_extra to standardize extra dependencies

[deepyaman]

Pull Request Check List

Resolves: #4123

• Added tests for changed code.
• Updated documentation for changed code.

[deepyaman]

I can't quite figure out why the two running jobs are failing; the tests pass on my machine (aside from venv-related ones that didn't pass even when I first cloned the project), and the timeouts seem to happen before tests run.

[finswimmer]

Thanks a lot for your contribution @deepyaman 👍

I've talked about it with @sdispater and he's fine with being not case sensitiv here. But instead of using pkg_resources.safe_extra it would be better to implement our own helper method. Thus we can avoid a dependency that isn't absolutely necessary. The implementation seems to be trivial: https://github.com/pypa/pkg_resources/blob/6f81a44010d1266494025647dd1e1f0befa5b26b/pkg_resources/__init__.py#L1435

fin swimmer

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

[deepyaman]

But instead of using pkg_resources.safe_extra it would be better to implement our own helper method.

@finswimmer Sorry for the delay, but this is done!

[neersighted]

This looks good to me -- can we rebase this so it can be merged?

[neersighted]

Looks like we still need to get CI working -- also there's a lot of junk commits. Could we squash this PR down to 1-3 (logical/meaningful) commits?

[deepyaman]

Looks like we still need to get CI working -- also there's a lot of junk commits. Could we squash this PR down to 1-3 (logical/meaningful) commits?

Sorry, I had to step out, but I was in the middle of setting up my local env to resolve the issues. It's been a year, so a lot has changed, including my laptop. :)

I'll ping you once I've got them resolved, and I can also squash down to a change or two (I wrongly assumed or forgot that Poetry doesn't just squash-merge everything).

[deepyaman]

@neersighted Think we're good now! I'll just update with rebase since there's been more pushes, but I don't imagine there will be any issues.

[deepyaman]

Looks like we still need to get CI working -- also there's a lot of junk commits. Could we squash this PR down to 1-3 (logical/meaningful) commits?

As an unrelated note on this, looking at the (recent) commit history, there's a mix of Conventional Commit and old-school sentence-case commits. Maybe it makes sense to standardize on one?

[neersighted]

Looks like we still need to get CI working -- also there's a lot of junk commits. Could we squash this PR down to 1-3 (logical/meaningful) commits?

As an unrelated note on this, looking at the (recent) commit history, there's a mix of Conventional Commit and old-school sentence-case commits. Maybe it makes sense to standardize on one?

We're not picky yet but eventually I'll get around to gating commits on conventional commit compliance 😄

And we do squash merge when needed, but during a more involved change (with a developer that knows how to rebase) like this I prefer to have logical commits to make bisecting easier -- e.g. in this case I'd prefer add tests then add code to make the change.

[deepyaman]

I'd prefer add tests then add code to make the change.

@neersighted Do you want me to do this, or is this good to go as is? I've never broken commits down like that (because generally try to ensure that tests pass on each commit on the main branch on projects I've worked on), but I can update it if that's the standard for Poetry and what we need to close this out.

[neersighted]

I'd prefer add tests then add code to make the change.

@neersighted Do you want me to do this, or is this good to go as is? I've never broken commits down like that (because generally try to ensure that tests pass on each commit on the main branch on projects I've worked on), but I can update it if that's the standard for Poetry and what we need to close this out.

It's nothing enforced -- merely a preference for review, and given we're already reviewed this change one commit is fine 😄

stale

[deepyaman]

@neersighted Thank you so much for your help in closing this out!

[github-actions]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.