search

OCA / l10n-finland

GNU Affero General Public License v3.0
4 stars 21 forks source link

[10.0][ADD] business code and validation #15

Closed jarmokortetjarvi closed 6 years ago

jarmokortetjarvi commented 6 years ago

Adds a business id for partners. Split to two modules: one just adds the field and other adds validation and formatting. This way the validation will be optional, and the module can be installed in environments with incorrectly formatted business ids

mlaitinen commented 6 years ago

You forgot oca_dependencies.txt file from the repo root

coveralls commented 6 years ago

Coverage Status

Coverage decreased (-51.6%) to 48.387% when pulling 62d478fe79ddd6e1909211ab439331a1c1afbf94 on Tawasta:10.0-l10n_fi_business_code into 5aee1230995891cfd13ee76f6afc11fe0eb87273 on OCA:10.0.

coveralls commented 6 years ago

Coverage Status

Coverage decreased (-3.3%) to 96.667% when pulling c10bf1308b18cbeb3eed3067437892d38d3639eb on Tawasta:10.0-l10n_fi_business_code into a504b07086d8d5c6fb6a64a2807ecc4e7b8754de on OCA:10.0.

mlaitinen commented 6 years ago

I guess the validation module could use one or two unit tests

jarmokortetjarvi commented 6 years ago

@mlaitinen that is true

aisopuro commented 6 years ago

OK, LGTM. The code coverage says you are (at least) not covering some validation cases, such as validating the empty string and an RY-code. Could you see if you could manage to kick the coverage up to a 100 by covering those cases as well?

jarmokortetjarvi commented 6 years ago

Sure. I'll add the missing tests

mlaitinen commented 6 years ago

What are those failed codecov checks? I don't understand them.

Also, let's respect the OCA policy for requiring 2 approved reviews per PR in the future.

jarmokortetjarvi commented 6 years ago

Accidentally pushed to the upstream :scream: Did a rollback for that. New pull request coming soon...

jarmokortetjarvi commented 6 years ago

And here we go #16

Also added the missing branch protection rules to main branches to prevent this kind of mistake