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.

Sign up for GitHub

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

Jump to bottom

[ADD] pingen_env #327

Merged
merged 1 commit into from
Jul 8, 2024
Merged

[ADD] pingen_env #327

merged 1 commit into from
Jul 8, 2024

Conversation

ajaniszewska-dev
Copy link

@ajaniszewska-dev ajaniszewska-dev commented Apr 28, 2023 •
edited
Loading

we want to manage pingen config values via server environment

@ajaniszewska-dev ajaniszewska-dev force-pushed the 16_pingen_env branch 3 times, most recently from 0d3e492 to 37be778 Compare May 2, 2023 13:25
grindtildeath
grindtildeath reviewed May 2, 2023
View reviewed changes
pingen_env/models/res_company.py Outdated Show resolved Hide resolved
pingen_env/views/base_config_views.xml Outdated Show resolved Hide resolved
grindtildeath
grindtildeath approved these changes May 3, 2023
View reviewed changes
pingen_env/hooks.py Outdated Show resolved Hide resolved
TDu
TDu approved these changes May 4, 2023
View reviewed changes
pingen_env/__manifest__.py Outdated Show resolved Hide resolved
@github-actions
Copy link

github-actions bot commented Sep 3, 2023

There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

@github-actions github-actions bot added the stale PR/Issue without recent activity, it'll be soon closed automatically. label Sep 3, 2023
@grindtildeath
Copy link
Contributor

@ajaniszewska-dev can you please fix the tests? 🙏

@github-actions github-actions bot removed the stale PR/Issue without recent activity, it'll be soon closed automatically. label Sep 17, 2023
@ajaniszewska-dev ajaniszewska-dev force-pushed the 16_pingen_env branch 4 times, most recently from 56b1cd9 to 4da3ee2 Compare October 9, 2023 08:28
@github-actions GitHub Actions
Copy link

There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

@github-actions github-actions bot added the stale PR/Issue without recent activity, it'll be soon closed automatically. label Feb 11, 2024
@ajaniszewska-dev ajaniszewska-dev force-pushed the 16_pingen_env branch 3 times, most recently from 6ba73ee to 848a89c Compare February 26, 2024 13:02
@ajaniszewska-dev
Copy link
Author

@pedrobaeza can we merge? 🙏

pedrobaeza
pedrobaeza reviewed Feb 26, 2024
View reviewed changes
pingen_env/README.rst Outdated Show resolved Hide resolved
pedrobaeza
pedrobaeza reviewed Feb 26, 2024
View reviewed changes
pingen_env/hooks.py Outdated
def post_init_hook(cr, registry):
# When installing pingen_env we want to delete data from DB
cr.execute(
"""UPDATE res_company
Copy link
Member

Choose a reason for hiding this comment

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

This seems a solution of your current problem in existing DBs instead of a generic one.

Copy link
Author

Choose a reason for hiding this comment

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

@pedrobaeza updated readme + removed hook, i think we should be ok now

@ajaniszewska-dev ajaniszewska-dev force-pushed the 16_pingen_env branch 2 times, most recently from 85cbf4a to b742427 Compare February 27, 2024 13:58
@pedrobaeza pedrobaeza added this to the 16.0 milestone Feb 27, 2024
pedrobaeza
pedrobaeza reviewed Feb 27, 2024
View reviewed changes
pingen_env/readme/USAGE.rst Outdated
Define values under global section name called 'pingen', or in case of different config per company - global section name connected with company name:


[pingen.companyA]
Copy link
Member

Choose a reason for hiding this comment

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

Isn't there any problem with spaces in the company name or any other special character not correctly being parsed by configuration helper?

And in any case, it should be more descriptive if you put a real demo company name:

Suggested change
[pingen.companyA]
[pingen.My Company (San Francisco)]

Copy link
Author

Choose a reason for hiding this comment

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

agree company name maybe not the best way to differentiate, but we use it in prod env for many months already so for now ill leave it as it is.
Updated readme.

Copy link
Contributor

Choose a reason for hiding this comment

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

but... please add a todo to use a tech name. This can be fragile.

@ajaniszewska-dev ajaniszewska-dev force-pushed the 16_pingen_env branch 2 times, most recently from d81e1d2 to f692906 Compare March 1, 2024 09:53
@github-actions github-actions bot removed the stale PR/Issue without recent activity, it'll be soon closed automatically. label Mar 3, 2024
simahawk
simahawk reviewed Mar 4, 2024
View reviewed changes
pingen_env/readme/USAGE.rst Outdated
Define values under global section name called 'pingen', or in case of different config per company - global section name connected with company name:


[pingen]
Copy link
Contributor

Choose a reason for hiding this comment

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

this must be wrapped in code block

@github-actions GitHub Actions
Copy link

github-actions bot commented Jul 7, 2024

There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

@github-actions github-actions bot added the stale PR/Issue without recent activity, it'll be soon closed automatically. label Jul 7, 2024
@ajaniszewska-dev
Copy link
Author

@pedrobaeza good to merge?

@pedrobaeza
Copy link
Member

@simahawk must say, as he has comments.

@simahawk
Copy link
Contributor

simahawk commented Jul 8, 2024

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 16.0-ocabot-merge-pr-327-by-simahawk-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit a691a8e into OCA:16.0 Jul 8, 2024
4 of 7 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 19abb3f. Thanks a lot for contributing to OCA. ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pedrobaeza pedrobaeza pedrobaeza left review comments

@TDu TDu TDu approved these changes

@grindtildeath grindtildeath grindtildeath approved these changes

@simahawk simahawk simahawk approved these changes

Assignees
No one assigned
Labels
approved merged 🎉 stale PR/Issue without recent activity, it'll be soon closed automatically.
Projects
None yet
Milestone
16.0
Development

Successfully merging this pull request may close these issues.
6 participants
@ajaniszewska-dev @grindtildeath @pedrobaeza @simahawk @OCA-git-bot @TDu