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

Automatically commit blacklisted ports #7

Merged
merged 2 commits into from
Oct 11, 2022

Conversation

sebalix
Copy link
Collaborator

@sebalix sebalix commented Aug 31, 2022

This change is not backward-compatible.

This feature will store blacklisted PR or module migration in a ./oca-port/blacklist/{MODULE}.json file (one file per module) and this will be committed in the working branch so this information will be shared with other contributors (PR or module blacklisted won't be listed anymore, but the reason why it was blacklisted will be displayed instead).

Here is a demo of a blacklisted PR with the current changes: OCA/wms#464
image

fix #4

@sebalix sebalix requested a review from simahawk August 31, 2022 15:47
@sebalix
Copy link
Collaborator Author

sebalix commented Aug 31, 2022

I wonder if we should add a comment when we blacklist something, explaining the reason why we did it (changes already ported, lint, doesn't apply anymore, module replaced by another one or renamed...)

@sebalix sebalix added the enhancement New feature or request label Sep 1, 2022
Copy link

@ivantodorovich ivantodorovich left a comment

Choose a reason for hiding this comment

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

I suggest to store this file in .oca/oca-port/blacklist/{MODULE}.json (or similar)
Goal is to put it under an .oca directory that we can use for (possible) future oca tools

Also maybe use a yml file instead of json, that's more common than json in this kind of tools

@sebalix
Copy link
Collaborator Author

sebalix commented Sep 7, 2022

@ivantodorovich good ideas. For YAML the only drawback is to introduce a new dependency, while JSON is part of the Python std lib.

@simahawk
Copy link
Contributor

simahawk commented Sep 8, 2022

I agree on the path, I agree on keeping JSON. And, yes, let's collect a comment!

@sebalix
Copy link
Collaborator Author

sebalix commented Sep 8, 2022

@simahawk I'll try to pop-up the editor during the git commit, or add a user input in the commit message. I don't think we need to store this in the JSON file.

@sebalix
Copy link
Collaborator Author

sebalix commented Oct 10, 2022

Here is a demo of a blacklisted PR with the current changes: OCA/wms#464

@sebalix sebalix force-pushed the imp-inputstorage branch 2 times, most recently from e7ea359 to a263bcd Compare October 10, 2022 11:29
@sebalix sebalix added this to the 0.10 milestone Oct 10, 2022
@sebalix
Copy link
Collaborator Author

sebalix commented Oct 11, 2022

I'm quite happy with these changes at the moment, and want to build a new release for that, so... merging :)

@sebalix sebalix merged commit 1d0377f into OCA:main Oct 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request needs review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

InputStore: store module and PRs to blacklist in one file per module
3 participants