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

Adding additional questions to copier template for use outside of the OCA #144

Merged
merged 1 commit into from
Jul 18, 2022

Conversation

theangryangel
Copy link
Member

@theangryangel theangryangel commented Jul 12, 2022

The OCA addons repo template is a great resource for all partners, it allows us to build with the possibility of easily offering new modules to the OCA in the future by aligning as close as reasonably possible to the OCA's standards from the outset.

To ease this process we're proposing a few additional questions in the copier template that can disable certain areas that are problematic.

We have set all answers to default to ensure no change in current behaviour. We have setup all (except github_enable_stale_action) questions to only be asked when ci == 'GitHub'.

  • github_check_license: Check the manifest license
  • github_enforce_dev_status_compatibility: Disable the development status, applicable for early work which legitimately may be alpha/pre-alpha
  • github_enable_codecov: Disable the codecov action
  • github_enable_makepot: Disable the update of pot files, for not-yet-open-source repositories, this functionality currently fails
  • github_enable_stale_action: Disable the GitHub stale action

By introducing these questions it allows usage for outside of the OCA without needing to edit github workflows directly.

I've raised this as a draft for discussion, as I'm sure changes will need to be made, tests added, etc. But before we add those I wanted to check if this has the possibility of being accepted, or if it's completely out of scope of the project :)

cc @GlodoUK T3227

@legalsylvain
Copy link
Contributor

Divided opinion. I'm un favor to have OCA tools that can be used for other repo. That promote OCA standards and possibly generate new contributors for OCA tools.
For that reason, i'm OK to integrate makepot questions (useless out of the OCA and maybe in some OCA repo like openupgrade) and codecov that generate extra noice (and there is codecov alternative)
For the two others topics i dont understand why introduce extra questions (and so complexity)
Why not follow the OCA rules ?

@sbidoul
Copy link
Member

sbidoul commented Jul 13, 2022

I'm in favor. These represent a low maintenance cost for OCA and make the template more versatile.

Making the dev status check optional is also important for older OCA branches where making it mandatory would make many repos red, generating unncessary busywork. Could you add a question to enable the license check too for the same reason, and so it closes #83 .

copier.yml Outdated Show resolved Hide resolved
copier.yml Outdated Show resolved Hide resolved
copier.yml Outdated Show resolved Hide resolved
copier.yml Outdated Show resolved Hide resolved
@legalsylvain
Copy link
Contributor

Making the dev status check optional is also important for older OCA branches where making it mandatory would make many repos red, generating unncessary busywork. Could you add a question to enable the license check too for the same reason, and so it closes #83 .

I didn't thought about old OCA branches. It's a 👍

@sbidoul
Copy link
Member

sbidoul commented Jul 13, 2022

In the same vein, I created #145.

@theangryangel
Copy link
Member Author

theangryangel commented Jul 13, 2022

For the two others topics i dont understand why introduce extra questions (and so complexity)

For the stale question, for me, it's 100% selfish, and I'd understand it if that's not something the OCA want to accept.

We don't use GitHub issues for 99% of the repositories until they're open source, and even then the traffic we get on them is currently so low - if we don't need the action running I'd prefer to try and save the planet, even if it's just a very small amount :)

Could you add a question to enable the license check too for the same reason, and so it closes #83 .

Done :)


I've adjusted the implementation so that the GitHub stale question doesn't depend on if ci == 'GitHub', I assumed there are scenarios where Travis is used, but stale does need to be enabled?

I've also tried to work around what I believe is python-poetry/poetry#3010 which was causing the Python 2.7 tests to intermittently fail on GitHub runners by slightly adjusting the repo's own test.yml.

Do you want me to add further tests for coverage?

@theangryangel theangryangel marked this pull request as ready for review July 13, 2022 08:51
- name: Check development status
run: manifestoo -d . check-dev-status --default-dev-status=Beta
- name: Initialize test db
run: oca_init_test_database
{%- if not github_enforce_dev_status_compatibility %}
if: always()
Copy link
Member

Choose a reason for hiding this comment

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

Is it not continue-on-error instead ?

Can you do the same for the license check ?

And ideally show a test run somewhere ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Is it not continue-on-error instead ?

Apologies I thought that was only available at the job level.

Can you do the same for the license check ?

Added

And ideally show a test run somewhere ?

I'll set something setup now :)

… used outside of the OCA more easily, fixes T3227
@theangryangel
Copy link
Member Author

theangryangel commented Jul 13, 2022

Some sample runs are available at https://github.com/GlodoUK/oca_addons-repo-template-pr-144/pulls against 2 different branches, with slightly different settings.

type: bool
default: yes
help: GitHub action checks the minimum development status?
when: &ci_is_github "{{ ci == 'GitHub' }}"
Copy link
Member

Choose a reason for hiding this comment

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

❤️

@@ -54,6 +54,9 @@ jobs:
key: >-
cache ${{ secrets.CACHE_DATE }} ${{ env.PY }} ${{ runner.os }} ${{
hashFiles('pyproject.toml') }} ${{ hashFiles('poetry.lock') }}
# HACK python-poetry/poetry#3010 and 3336 workaround for GitHub actions
# and poetry 1.1.2 throwing 'Incorrectly nested style tag found.'
- run: poetry config experimental.new-installer false
Copy link
Member

Choose a reason for hiding this comment

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

I can't help thinking that poetry is overkill for this project.

@sbidoul sbidoul merged commit f0059e3 into OCA:master Jul 18, 2022
@theangryangel
Copy link
Member Author

Thanks Stéphane, much appreciated <3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants