Making it run again#54
Conversation
The leadin `./` don't work with VSCode. Also alphabetically sorted. Signed-off-by: Paul Spooren <mail@aparcar.org>
Create contract before creating the invoices. Also print the mail address in the overview. Signed-off-by: Paul Spooren <mail@aparcar.org>
Use Pathlib to simplify template code. Also send all outgoing invoices to BCC as `rechnung` does not store mail in a *Sent* folder. Signed-off-by: Paul Spooren <mail@aparcar.org>
Using Umlaute in templates or yaml files caused trouble, now handle everything as UTF-8. Signed-off-by: Paul Spooren <mail@aparcar.org>
Use the `locale` module to print the correct currency instead of some self created format. Signed-off-by: Paul Spooren <mail@aparcar.org>
Decreses time wasted during debugging. Signed-off-by: Paul Spooren <mail@aparcar.org>
cfra
left a comment
cfra
left a comment
There was a problem hiding this comment.
I have looked over it and all in all it seems like a nice cleanup. 🙂
There are a a few minor comments inline.
| assets/ | ||
| bin/ | ||
| contracts/ | ||
| docs/_build/ | ||
| invoices/ | ||
| lib* | ||
| pyvenv.cfg | ||
| settings.yaml |
There was a problem hiding this comment.
Checking the current .gitignore with the ./ lead-in using git check-ignore --verbose, it seems indeed like these rules are broken and do never match.
However, it does not really seem like a good idea to me to add things like contracts/ or invoices/ to .gitignore without making them relative to the repository root:
Imagine you are changing the contract.py module in rechnung into its own contracts package.
With your change, this whole package would be ignored by git, because any directory that goes by the name of contracts will be ignored anywhere in the repository.
The better solution might be to change all occurrences of ./ in the .gitignore with /:
- It's an allowed pattern
- It seems to match the originally intended meaning
- It doesn't ignore directories with common names broadly and unspecifically
| contract = yaml.safe_load( | ||
| (settings.contracts_dir / filename).read_text("utf-8") | ||
| ) |
There was a problem hiding this comment.
Reading everything into a buffer to then passing it to yaml.safe_load seems rather inelegant, as yaml.safe_load also accepts a stream.
Instead of using the read_text method of pathlib and duplicating everything in memory, the encoding problems could probably also be solved like this:
| contract = yaml.safe_load( | |
| (settings.contracts_dir / filename).read_text("utf-8") | |
| ) | |
| with open(Path(settings.contracts_dir / filename), "r", "utf-8") as contract_file: | |
| contract = yaml.safe_load(contract_file) |
There was a problem hiding this comment.
After all I think the encoding was partly a problem on my side. I think some Python files where for whatever reason in a different encoding causing a lot of trouble. Should I fix this particular issue or should I revert it?
| (settings.contracts_dir / filename).read_text("utf-8") | ||
| ) | ||
|
|
||
| if not contract.get("active", False): |
There was a problem hiding this comment.
As this seems like a newly introduced feature, wouldn't it be better to default to a contract being active, so that not all existing active contracts need to be explicitly marked as active, but instead only the new inactive contracts need to be marked as inactive?
| if not contract.get("active", False): | |
| if not contract.get("active", True): |
If no clear state of `active` is given, guess that they are active as this is a new setting and may be missing in existing setups. Signed-off-by: Paul Spooren <mail@aparcar.org>
Signed-off-by: Paul Spooren <mail@aparcar.org>
2 participants
Not touched for a long time, trying to run it introduced a whole bunch of problem. Here are some solved.