-
-
Notifications
You must be signed in to change notification settings - Fork 303
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
[10.0] l10n_it_fatturapa: xml doctor for fatturapa #1218
Conversation
@eLBati travis ha ricominciato a rompersi su roba che non ci incastra |
se i problemi non riguardano righe modificate da te, puoi ignorarli |
è abbastanza randomico |
@eLBati mi sembra ok, ora sistemo il test per controllare il risultato
|
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.
Anche
l10n_it_fatturapa/bindings/fatturapa.py:8:1: F403 'from binding import *' used; unable to detect undefined names
l10n_it_fatturapa/bindings/fatturapa.py:42:20: F405 '_root' may be undefined, or defined from star imports: binding
l10n_it_fatturapa/bindings/fatturapa.py:44:23: F405 '_root' may be undefined, or defined from star imports: binding
l10n_it_fatturapa/bindings/fatturapa.py:49:20: F405 '_root' may be undefined, or defined from star imports: binding
l10n_it_fatturapa/bindings/fatturapa.py:51:23: F405 '_root' may be undefined, or defined from star imports: binding
l10n_it_fatturapa/bindings/fatturapa.py:57:25: F405 '_root' may be undefined, or defined from star imports: binding
l10n_it_fatturapa/bindings/fatturapa.py:58:66: E261 at least two spaces before inline comment
l10n_it_fatturapa/bindings/fatturapa.py:80:23: F405 'CreateFromDocument' may be undefined, or defined from star imports: binding
non posso mettere a mano 500 imports :D c'è posso dire a flake di ignorare la cosa? per E261 è una # di refuso |
Come mai 500? Non basterebbero |
parlavo del wild import (ho messo ignore), _root è stupido flake non si accorge che lo assegno sotto |
si è intrippato travis, da errori differenti.. :( |
Che in locale non ottieni? |
In locale ho sempre casino, cmq ho ri-pushato credo che aggiungesse un '\n\n' alle inconsistenze che a qualche test non piaceva |
sembra ok ora, almeno la mia roba |
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.
Per il resto OK per me
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.
Grazie della PR!
Solo qualche commento non bloccante
target[path] = mandatory | ||
else: | ||
assert target[path] == mandatory, 'Element already present with ' \ | ||
'different minOccurs valus' |
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.
'different minOccurs valus' | |
'different minOccurs value' |
Si potrebbe anche aggiungere il path
(e volendo anche il text
) per uniformarlo agli altri messaggi di warning e poter capire dai log che sta succedendo
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.
ma è un assert, cmq dimmi cosa scrivo, ce lo metto, non dovrebbe capitare
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.
if target[path] != mandatory:
msg = "Element having path %s already present with a different minOccurs value" % path
_logger.warn(msg)
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.
no deve proprio scoppiare se capita :)
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.
Va bene, secondo me è utile indicare il path di cosa sta fallendo nei log per capire cosa sta succedendo e dove.
Se usarlo per sollevare eccezione o con un assert decidi tu
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.
si ora riproduco aggiungendo un doppione nell'xsd e vedo come viene meglio l'errore
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.
credo una cosa simile sia accettabile:
Element //DatiTrasporto/Descrizione is already present with different minOccurs value
it fixes: - removes xs:dateTime if bogus and not mandatory i.e. 0001-01-01T00:00:00.000+02:00 where python raises OverflowError - removes timezone from xs:date to make pyxb able to compare with 1-1-1970, it also removes the need of patching pyxb - removes space only strings if not mandatory, else replace with a dash breaking change: modules needs to import binding.fatturapa instead of bindings.fatturapa_v_1_2, this would be asl useful for new specs
proposal per #1172
it fixes:
i.e. 0001-01-01T00:00:00.000+02:00 where python raises
OverflowError
1-1-1970, it also removes the need of patching pyxb
breaking change:
modules needs to import binding.fatturapa instead of
bindings.fatturapa_v_1_2, this would be usefull also for
new specs
--
Confermo di aver firmato il CLA https://odoo-community.org/page/cla e di aver letto le linee guida su https://odoo-community.org/page/contributing