search

SergelsOrg / csv2tex

Tool to replace placeholders in tex-files
GNU General Public License v3.0
5 stars 0 forks source link

Endyear #109

Closed DonMischo closed 2 years ago

DonMischo commented 2 years ago

code changes for endyear updated tex files

ArchibaldBienetre commented 2 years ago

So, if this were a real job, and you were my intern, your grades would actually be pretty bad. :disappointed:

I'm a bit disappointed, but I'll spare you the rant.

I'd ask you kindly to give it a try and fix those failing tests.

Any git branching / rebasing issues, we'll fix together then after.

DonMischo commented 2 years ago

I had some failing tests for the exceptions, but last time it was cause by the selected language, so I didin't much attention. However, I checked the tests again and saw some tests not connected to the exceptions. I'll solved those tests as soon as possible. Most seem connected to the additional field. Locallly I added an accutual sample csv, today. Annother test passed. I'll check the other tests during the next days.

DonMischo commented 2 years ago

Fixed half of broken tests.

However, I'm not able to fix the heuristic tests... I think I traced down the issue to their roots. CsvValidator.java:66 record.size(); counts the empty column, too. So header and contents are always equal in length. Do I miss something?

ArchibaldBienetre commented 2 years ago

Ah, sorry, was out sick for a week and missed your (timely) reply.

I'll have a look asap (likely, Wednesday night as usual), will be on leave next week so I may jump ahead with the rebase.

ArchibaldBienetre commented 2 years ago

Feels like work :sweat_smile: image

ArchibaldBienetre commented 2 years ago

Merge conflicts are a lot, because an already-merged branch has been continued here, it seems: #97

ArchibaldBienetre commented 2 years ago

Worst merge I've had in months. I need to reconsider my approach...

Rebase does not look good, merge is a mess...

In general, it's always easier if it's one's own code.

:mag: I'll go through the code, probably refactor it a bit (e.g. split very long lines), then re-try the rebase.

ArchibaldBienetre commented 2 years ago

Successfully rebased it.

Now I'll look into the remaining failing tests :mag:

ArchibaldBienetre commented 2 years ago

Well, at least one thing: By not screwing up the CSV, I'm down to 2 failing tests total (local run). That was wrong AGAIN on this branch. Like so many times before (cf 2b4f130627128792de5572a719e9444415e7277d, for example).

The remaining broken test is ErfurtSchoolReportSmokeTest - it's broken because changes that were made on main were ignored by @implementer's branch.

ArchibaldBienetre commented 2 years ago

2 hours and 15 minutes of work. But now the tests should run again.

Now, I can have a closer look at the actual code changes soon... but maybe not tonight anymore.

ArchibaldBienetre commented 2 years ago

3 hours. I'm wiped out :sleepy:

@DonMischo please double-check the QA side (i.e., please run it and check if it still looks fine). I don't know if I broke the layout by accident :see_no_evil:

Then, go ahead and merge if you like :+1: