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

[WIP] checklog-odoo improvements #2

Merged
merged 20 commits into from
Apr 26, 2024
Merged

[WIP] checklog-odoo improvements #2

merged 20 commits into from
Apr 26, 2024

Conversation

baimont
Copy link
Contributor

@baimont baimont commented Oct 10, 2023

#1

@baimont baimont force-pushed the bai_checklog_odoo_imp branch 7 times, most recently from e9e0b68 to 4b50166 Compare October 10, 2023 11:39
tests/test_checklog.py Outdated Show resolved Hide resolved
@sbidoul
Copy link
Member

sbidoul commented Oct 10, 2023

A few quick comments

  • do we need tests/data/odoo/* ?
  • in the LICENSE and other files, the copyright is ACSONE only (but you remain an author, of course, and are credited for that in the project metadata)
  • remove __about__.py and use hatch-vcs for the version
  • there is no need for a test class, you can use simple functions in test_checklog.py

@baimont
Copy link
Contributor Author

baimont commented Oct 10, 2023

A few quick comments

  • remove __about__.py and use hatch-vcs for the version

@sbidoul trying to use hatch-vcs makes the installation impossible

10f1b02

❯ pip install -e .
Obtaining file:///home/benoit/Projects/checklog-odoo
Installing build dependencies ... done
Checking if build backend supports build_editable ... done
Getting requirements to build editable ... done
Preparing editable metadata (pyproject.toml) ... error
error: subprocess-exited-with-error

× Preparing editable metadata (pyproject.toml) did not run successfully.
│ exit code: 1
╰─> [20 lines of output]
Traceback (most recent call last):
File "/home/benoit/.virtualenvs/checklog-odoo/lib/python3.10/site-packages/pip/_vendor/pyproject_hooks/_in_process/_in_process.py", line 353, in
main()
File "/home/benoit/.virtualenvs/checklog-odoo/lib/python3.10/site-packages/pip/_vendor/pyproject_hooks/_in_process/_in_process.py", line 335, in main
json_out['return_val'] = hook(**hook_input['kwargs'])
File "/home/benoit/.virtualenvs/checklog-odoo/lib/python3.10/site-packages/pip/_vendor/pyproject_hooks/_in_process/_in_process.py", line 176, in prepare_metadata_for_build_editable
whl_basename = build_hook(metadata_directory, config_settings)
File "/tmp/pip-build-env-_vcf5t80/overlay/lib/python3.10/site-packages/hatchling/build.py", line 78, in build_editable
return os.path.basename(next(builder.build(wheel_directory, ['editable'])))
File "/tmp/pip-build-env-_vcf5t80/overlay/lib/python3.10/site-packages/hatchling/builders/plugin/interface.py", line 93, in build
self.metadata.validate_fields()
File "/tmp/pip-build-env-_vcf5t80/overlay/lib/python3.10/site-packages/hatchling/metadata/core.py", line 243, in validate_fields
_ = self.version
File "/tmp/pip-build-env-_vcf5t80/overlay/lib/python3.10/site-packages/hatchling/metadata/core.py", line 128, in version
self._version = self._get_version()
File "/tmp/pip-build-env-_vcf5t80/overlay/lib/python3.10/site-packages/hatchling/metadata/core.py", line 226, in _get_version
version = self.hatch.version.cached
File "/tmp/pip-build-env-_vcf5t80/overlay/lib/python3.10/site-packages/hatchling/metadata/core.py", line 1415, in cached
raise type(e)(message) from None
packaging.version.InvalidVersion: Error getting the version from source vcs: Invalid version: '1c63fc2af2c78c80c3eaa19541c508144b11bf1e'
[end of output]

note: This error originates from a subprocess, and is likely not a problem with pip.
error: metadata-generation-failed

× Encountered error while generating package metadata.
╰─> See above for output.

note: This is an issue with the package mentioned above, not pip.
hint: See above for details.

Any idea?

@sbidoul
Copy link
Member

sbidoul commented Oct 10, 2023

That's because there was a tag with a name that hatch-vcs does not support. I removed it.

@baimont baimont force-pushed the bai_checklog_odoo_imp branch 2 times, most recently from 0e9b170 to 7ea96e2 Compare October 13, 2023 07:54
@baimont
Copy link
Contributor Author

baimont commented Oct 13, 2023

@sbidoul

I tried

odoo -d mydb -i base --stop-after-init | checklog-odoo

and

odoo -d mydb -i base --stop-after-init | acsoo checklog

which both gave me

Error: No Odoo log record found in input.

And I believe this is not right, right?

@sbidoul
Copy link
Member

sbidoul commented Oct 13, 2023

@baimont that is because odoo logs on stderr and | reads the stdout of the process on the left.

That is one of the reaons we use unbuffer in CI, as it copies stderr to stdout.

So that is ok, but it may be worth mentioning in the README of this project.

@baimont
Copy link
Contributor Author

baimont commented Oct 26, 2023

Hello @sbidoul,

Je vais avoir un peu de temps prochainement. Ce serait l'occasion de retravailler ce sujet, si tu as un moment pour reviewer avant?

LICENSE.txt Outdated Show resolved Hide resolved
src/checklog_odoo/__init__.py Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
tox.ini Show resolved Hide resolved
@baimont
Copy link
Contributor Author

baimont commented Dec 6, 2023

@sbidoul, @ThomasBinsfeld asks me to finish this pull request. is something blocking the merge?

@sbidoul
Copy link
Member

sbidoul commented Dec 6, 2023

asks me to finish this pull request. is something blocking the merge?

@baimont I had not noticed you had handled the last review comments. We you do so, it's a good habit to leave a note on the PR saying it is ready to review again.

@baimont
Copy link
Contributor Author

baimont commented Dec 6, 2023

asks me to finish this pull request. is something blocking the merge?

@baimont I had not noticed you had handled the last review comments. We you do so, it's a good habit to leave a note on the PR saying it is ready to review again.

I used

image

which led to

image

What's the difference with a note? As I resolved all the open comments I was guessing it was enough.

@baimont
Copy link
Contributor Author

baimont commented Dec 6, 2023

@sbidoul ready to review again

@baimont
Copy link
Contributor Author

baimont commented Feb 28, 2024

@sbidoul ready to review again

hi there,
still the case

@acsone acsone deleted a comment from baimont Apr 5, 2024
@sbidoul
Copy link
Member

sbidoul commented Apr 26, 2024

Ok, I did some tyding up and I'll now merge. Thanks for your patience.

@sbidoul sbidoul merged commit 1e1652d into master Apr 26, 2024
6 checks passed
@sbidoul sbidoul deleted the bai_checklog_odoo_imp branch April 26, 2024 16:00
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.

2 participants