-
-
Notifications
You must be signed in to change notification settings - Fork 784
[18.0] [OU-ADD] l10n_es_igic: Merged into l10n_es #5378
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
base: 18.0
Are you sure you want to change the base?
Conversation
15e3c6e to
c8006a6
Compare
|
Have you checked if there's any need of XML-ID renaming of the existing taxes? |
No, sorry. I will check. Thanks! |
etobella
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks
| (["account_pymes_canary_296"], "account_pymes_296"), | ||
| (["account_pymes_canary_490"], "pgc_pyme_490"), | ||
| (["account_pymes_canary_551"], "pgc_pyme_551"), | ||
| (["account_pymes_canary_4707"], ""), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You have to remove this and the following as they are doing nothing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did this because I couldn’t find this in v18, so I don’t know what I must do. I added a comment in the description of this PR asking what I should do in this case. Is it a missing account, or should I mark it as removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this account doesn't make sense anymore.... it should be ported to the existent account account_common_canary_47071, but I am not sure which is the best way to handle this specific migration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found this, and I think it’s still valid. WDYT? https://www.boe.es/buscar/doc.php?id=BOE-A-1992-28829
cf185e5 to
31eccb2
Compare
| (["account_full_canary_2553"], "account_full_2553"), | ||
| (["account_full_canary_257"], "account_full_257"), | ||
| (["account_full_canary_466"], "account_full_466"), | ||
| (["account_full_canary_4707"], ""), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't make sense: or you remove them, or you map to something, as keeping them this way won't make anything.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm with you on this, I'm just asking for help to know what to map it against, since there's nothing in 18. As I see we're all in the same situation and from what you mentioned, I can just leave it unmapped, so I'll do that and not block the topic any further. Thanks.
31eccb2 to
36876d6
Compare
Migration script added. Four XML IDs are empty because I missed them in v18. I have to test it in a local database when my doubts have been solved.