Conversation
9c7318d to
5c3bab9
Compare
Codecov Report
@@ Coverage Diff @@
## main #43 +/- ##
===========================================
+ Coverage 60.97% 91.56% +30.59%
===========================================
Files 13 13
Lines 574 593 +19
===========================================
+ Hits 350 543 +193
+ Misses 224 50 -174
Continue to review full report at Codecov.
|
Julien00859
left a comment
There was a problem hiding this comment.
We should be careful inside integration test, e.g. for that git config --global user.name, IMO we should mock those values before running the tests.
Please, don't store the truth inside of targz archives, it makes it impossible to read/diff on github
| from incipyt.tools.setuptools import LINUX_MIN_PYTHON_VERSION | ||
|
|
||
|
|
||
| def make_archive(): |
There was a problem hiding this comment.
missing docstring, this test-file will be our integration-test showcase/template, it needs to be documented so people feel easy using it as a template for their own tests
There was a problem hiding this comment.
and please note: I understand nothing at this function, what the fuck is an isolated_filesystem? why the fuck you need to chdir and remove a targz file???
There was a problem hiding this comment.
where this function is used?
| shutil.unpack_archive( | ||
| pathlib.Path(__file__).parent / "setuptools.tar.gz", "archive" | ||
| ) |
There was a problem hiding this comment.
I've just noticed that the "truth" is inside a tar.gz file tests/test_tools/setuptools.tar.gz. I think it is a bad idea, if one day we change something to the setuptools plugin, we'll have to adapt this tgz file but we won't have a clear diff of what changed.
Today's me want to read (inside github) what's inside that tgz file, future's me want to read a diff (inside github) of what changed inside that tgz file. Please don't store it as a tgz, just upload the whole folder as-is.
|
|
||
|
|
||
| def diff_files(dircmp): | ||
| result = {str(pathlib.Path(dircmp.left) / f) for f in dircmp.diff_files} |
| from incipyt import project | ||
|
|
||
|
|
||
| def diff_files(dircmp): |
| path | ||
| for path in result | ||
| if not any( | ||
| re.match(pattern, path) for pattern in [r".*/\.env/.*", r".*/dist/.*"] |
There was a problem hiding this comment.
let's use or own .gitignore https://stackoverflow.com/a/22090594
There was a problem hiding this comment.
while I agree those need to be stored somewhere else (a list of globs, just as .gitignore does, is just fine), using incipyt own .gitignore seems ankward and error-prone to me
There was a problem hiding this comment.
also, I'm not sure a 3rd party lib is really needed when we already have glob in the stdlib
incipyt/tools/git.py
Outdated
| self._set_git_credential_email = False | ||
| self._set_git_credential_name = False |
There was a problem hiding this comment.
git doesn't use credentials, github does. user.name and user.email are not credentials
There was a problem hiding this comment.
a name that starts with a verb should be a function, not a boolean
incipyt/tools/git.py
Outdated
| :param workon: Work-on folder. | ||
| :type workon: :class:`pathlib.Path` | ||
| """ | ||
| if self._set_git_credential_email: |
There was a problem hiding this comment.
ok now I understand.
So the commit message should be:
feat(git): user.name defaults to AUTHOR_NAME
Make sure there is no global git user.name and user.email: make sure
`git config --global user.name` is empty, same of `user.email`. Create a
project using git and setuptools: the wizard prompts you for an author
name and email as it couldn't determine them via git. When the project
is created, the author name and email are set inside `setup.cfg` but the
local git configuration still lacks an user.name and a user.email.
In this commit when no git user name or email is set globally and that
one was provided to the wizard, it automatically update the config of
the new git repository to set the user.name and the user.email to the
AUTHOR_NAME and AUTHOR_EMAIL.
328f61f to
397edbc
Compare
|
first integration tests failing on windows CI is due to non platform-agnostic path matching at testing.py:23 (diff is |
incipyt/tools/git.py
Outdated
| :param workon: Work-on folder. | ||
| :type workon: :class:`pathlib.Path` | ||
| """ | ||
| git_user_email_return = commands.run( |
There was a problem hiding this comment.
I would isolate the "update git config" in a dedicated function
incipyt/tools/git.py
Outdated
| commands.run( | ||
| [ | ||
| "git", | ||
| "-C", | ||
| os.fspath(workon), | ||
| "config", | ||
| "user.email", | ||
| project.environ["AUTHOR_EMAIL"], | ||
| ] | ||
| ) |
There was a problem hiding this comment.
I veto that.
Running $ git -C {} config user.email {} shouldn't expand over 9 lines of code.
def git(args, workon, **kwargs):
args = ["git", "-C", os.fspath(workon), *args]
return commands.run(args, **kwargs)
git = partial(git, workon=workon)This work include two new utilities, one to create a `StringTemplate` out of a file located inside of the new `/templates` directory, the other to copy a template file directly to the project.
A software license is a file stored at the root of a project which explain what it is allowed to do with the project source code and under what conditions. e.g. a project is qualified as "open-source" when it is published with a license that allows to distribute and modify the project source code. Until now the license file generated by incipyt was a simple copyright notice which made it explicit that the current work was the intellectual property of its owner. Under internationnal laws, this forbid anyone from modifying the work without the express authorization of the author. In this work, we added a way so that the user can choose an open-source license instead of the copyright notice. We added `--license <license>` to the CLI arguments and bundled common open-source licenses such as MIT and GPL. The previous behavior, to simply include a copyright notice, is still possible thanks to `--license copyright`.
See https://docs.pytest.org/en/latest/how-to/fixtures.html#yield-fixtures-recommended pytest recommends using a yield construction to setup and teardown environments for tests. Here we use it to clean project.environ and project.structure to provide reliable initial project in any case.
d2cd54a to
12736ae
Compare
12736ae to
90a2f4e
Compare
90a2f4e to
387fbda
Compare
Integration tests for basic tools: setuptools, venv and git.
PR enabled to check all tests on the CI server, DO NOT MERGE yet.
Update versions for CI (GitHub Actions and pre-commit).