[17.0][MIG] auth_saml

[vincent-hatakeyama]

/ocabot migration auth_saml

[OCA-git-bot]

Sorry @vincent-hatakeyama you are not allowed to mark the addon tobe migrated.

To do so you must either have push permissions on the repository, or be a declared maintainer of all modified addons.

If you wish to adopt an addon and become it's maintainer, open a pull request to add your GitHub login to the maintainers key of its manifest.

[vincent-hatakeyama]

/ocabot merge nobump

[OCA-git-bot]

Sorry @vincent-hatakeyama you are not allowed to merge.

To do so you must either have push permissions on the repository, or be a declared maintainer of all modified addons.

If you wish to adopt an addon and become it's maintainer, open a pull request to add your GitHub login to the maintainers key of its manifest.

[vincent-hatakeyama]

No idea what the issue is as I am the maintainer of the module.

[vincent-hatakeyama]

@pedrobaeza Sorry to bother you but I have no idea who to contact about the bot not recognizing me as the module maintainer anymore. I thought of contacting oca/server-auth-maintainers but there’s no such group apparently.

[pedrobaeza]

IIRC, @legalsylvain was the one developing the feature of looking into the previous branch to allow maintainers to merge. Anyway, you should respect the OCA rules, requiring 2 reviews for the merge, unless this gets blocked:

https://github.com/OCA/odoo-community.org/blob/master/website/Contribution/CONTRIBUTING.rst#review

[legalsylvain]

@pedrobaeza Sorry to bother you but I have no idea who to contact about the bot not recognizing me as the module maintainer anymore. I thought of contacting oca/server-auth-maintainers but there’s no such group apparently.

You can find the info here : https://github.com/OCA/repo-maintainer-conf/blob/master/conf/repo/server.yml#L1 And here : https://github.com/OCA/repo-maintainer-conf/blob/master/conf/psc/tools.yml#L1

No idea what the issue is as I am the maintainer of the module.

Indeed... https://github.com/OCA/server-auth/blob/16.0/auth_saml/__manifest__.py#L10

IIRC, @legalsylvain was the one developing the feature of looking into the previous branch to allow maintainers to merge.

@sbidoul what is the value of MAINTAINER_CHECK_ODOO_RELEASES in production ? (Should be a list from 8.0 to 17.0)

https://github.com/OCA/oca-github-bot/blob/master/environment.sample#L86

[legalsylvain]

No idea what the issue is as I am the maintainer of the module.

Investigated. You propose a change in a file you dont maintain. (Requirements.txt at repo foot) That's a limitation of the current algorythm.

[vincent-hatakeyama]

No idea what the issue is as I am the maintainer of the module.

Investigated. You propose a change in a file you dont maintain. (Requirements.txt at repo foot) That's a limitation of the current algorythm.

I believe that change is from previous commits kept by the migration process. The file is automatically generated, I suppose the changes should be removed in the migration commit so that the file is regenerated by a bot with the content?

[pedrobaeza]

I think requirements.txt and test_requirements.txt should be excluded from the algorithm.

[sbidoul]

I think requirements.txt and test_requirements.txt should be excluded from the algorithm.

This makes sense.

[gurneyalex]

/ocabot merge nobump

[OCA-git-bot]

This PR looks fantastic, let's merge it! Prepared branch 17.0-ocabot-merge-pr-634-by-gurneyalex-bump-nobump, awaiting test results.

[OCA-git-bot]

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

[OCA-git-bot]

@gurneyalex your merge command was aborted due to failed check(s), which you can inspect on this commit of 17.0-ocabot-merge-pr-634-by-gurneyalex-bump-nobump.

After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red.

[vincent-hatakeyama]

667 needs to be cherry picked to fix the tests.

Taking #652 would avoid having to forward port those changes separately too.

[astirpe]

@vincent-hatakeyama thank you! Would you check if it's all good now?

[bizzappdev]

IMO Commits [MIG] auth_saml: migrate to V17 and [MIG] auth_saml: fix E501 Line too long should be squashed into one.

The code LGTM.

[vincent-hatakeyama]

The tests now pass so that part is fine.

I think the lower test cover can be ignored.

I can’t merge the changes as I do not have the enough rights, but to me it’s ready to be merged in 17.0.

[sweiland57]

Will this be merged anytime soon ?

[leemannd]

Hello, the migration looks good. Usually we are also requested to squash administrative/translation commits

[astirpe]

Commits squashed. Thanks!

[PCatinean]

@gurneyalex could you please help us once again with a merge command?

[oussjarrousse]

I would like to push this merge request above the finish line... It seems that just more test coverage is required at the moment. I am not familiar with Odoo testing. How do you suggest I contribute? Where should I start?

[oussjarrousse]

Hello @astirpe. Any help I can provide?

[astirpe]

Can this one be merged?

@oussjarrousse thank you for asking, feel free to contribute in whatever you like!

[oussjarrousse]

@astirpe

Alright, It's the first time I write test code for Odoo, so I am still setting up the dev environment and figuring out where things are. If you have a good guide on that. I would appreciate it. I will keep you updated.