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

python: Add misc packages to default_module_mapping #17390

Merged
merged 2 commits into from
Oct 30, 2022

Conversation

cognifloyd
Copy link
Member

This adds several packages to default_module_mapping like @Eric-Arellano suggested in StackStorm/st2#5789 (comment)

For the oslo.* packages, I only need oslo.config, but I figured I'd add the rest of them as well. I looked on this page to find all the oslo.* packages.

@kaos
Copy link
Member

kaos commented Oct 28, 2022

From the failing test:

E AssertionError: Please update DEFAULT_MODULE_MAPPING to use canonical project names
E assert equals failed
E 'oslo.cache' 'oslo-cache'

@benjyw
Copy link
Contributor

benjyw commented Oct 29, 2022

In the past we've categorized these either as internal or "user api change". I think the latter is technically valid, since there is a user-facing impact? But I wouldn't call it a new feature.

@cognifloyd
Copy link
Member Author

None of the categories made much sense here. It doesn't break the user API and it's not a new feature. It's additional piece of config users can take advantage of... I just don't know. Feel free to change the category.

@benjyw
Copy link
Contributor

benjyw commented Oct 29, 2022

Yeah, none of the categories make much sense in this case. But we do want people to notice this in the changelog, so internal is probably not the best.

@kaos
Copy link
Member

kaos commented Oct 29, 2022

I agree it's hard to call this a new feature.. but I think it is. It is a new feature of Pants that you no longer need to declare the module mappings for these distributions in your own BUILD files any more.
We (including me) tend to consider features = functionality, but if we consider feature = behaviour this makes it a better fit, imho.

Edit: looking at some of the other user-api-changes PRs, I think that it would be helpful as a user, if changes under that name are mostly disruptive. That will have less noise to go through when upgrading to find changes that may need extra care. Maybe..?

@kaos kaos merged commit d05c437 into pantsbuild:main Oct 30, 2022
@Eric-Arellano Eric-Arellano added category:internal CI, fixes for not-yet-released features, etc. and removed category:user api change labels Oct 31, 2022
@stuhood stuhood mentioned this pull request Nov 19, 2022
@cognifloyd cognifloyd deleted the py-mod-map branch June 14, 2023 20:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend: Python Python backend-related issues category:internal CI, fixes for not-yet-released features, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants