- User Since
- Wed, Sep 2, 11:39 AM (3 w, 1 d)
clearly states what is object identity and that it does not change and that we compare it with is.
Wanted also to rename constructor parameters but they are tied to the config parameters.
@zack @olasd @vlorentz I am puzzled. Do I go with the ImmutableDict solution or throw away another part of what I did the last 3 days (did it also in another commit)? Already removed code that was against the strict signature checker in the unit tests.
@vlorentz: true, this is bad. I added the check after, and didn't check this...
And look! I can get rid of the identity check, just it would merge with itself. This one is to avoid perf penalty.
The fact that is not so common a pattern does not make it bad.
It IS exactly like using None if we make this dict immutable, with added benefits of readability and checking. It even has no performance penalty or anything.
I view it that way:
- identity checking a default value is always good, even if mutable, precisely because static values like these can never change identity (identity/value dichotomy). It would be wrong to value equality check it through if it is mutable of course;
- this value can only be modified inside the module (modulo import time hacking), so the question are:
- do we want it immutable (we don't plan to have code that mutate it)?
- trust the module code for well behaving? it is a real policy question, I think your answer is no
- this pattern is equivalent to the None one, except we statically (as opposed to manually replace None by a default) never allow any invalid type like NoneType. I try to leverage static typing to the fullest
After this rationale, which could be a valid policy to introduce in the project, now the proposition is:
Found how to do write passtrough functions while having good default handling and keep the signature checker happy. A bit hacky.
Fix typo and remove TODO comment
build failed and one thing to rework
Tue, Sep 22
Mon, Sep 21
I think you should not remove this code as it has one use case (filename is provided as parameter, from CLI or not) but have a different function name for the "prod" use case (e.g. parse_config_file_envvar).
Some parts in the old code are not needed anymore, but not all. In another diff we will cover the other use case.
For now better to have 2 functions.
Fri, Sep 18
Thu, Sep 17
Wed, Sep 16
Tue, Sep 15
dedented debian packages and updated one description
Also, is the grouping and the description accurate?
Mon, Sep 14
A message body is descriptive, the intent is different from the subject line, so it does not require imperative mood, as far as I know.
Commit bodies don't particularly use the imperative mood in the repo.
I mentioned the CLI functionality rather than the file, which I could also reformulate as "swh-scanner's CLI", but as your prefer I will use cli.py to be explicit about the location of the code.
Fri, Sep 11
That's just typing, so it could just theoretically crash mypy with non-POSIX paths.
In windows there are MS paths and POSIX paths, depending on the API used.
I don't think it may actually pose a problem because the distinction only exists at runtime, it is just... semantically invalid.
Only included commit about config
rebased 2 times master and refactored commits, improving typing
Thu, Sep 10
Separated commits, improved config loading
Wed, Sep 9
Fri, Sep 4
Thu, Sep 3
Changed to install pre-commit as a python package in venv.
Removed PATH adjusting instructions since they are no longer needed here.
The idea is to install it outside of a venv, to access it the same like the system package.
But I chose to install it as user (no pip install --system) because I think it shouldn't need to reside in root territory.
We can discuss the preferred approach :
- system or user?
- necessarily py3?
I don't think it needs to be py3 because instructions mentioned the Debian package which is py2 I think.