Freeciv

Any comment on this, Sveinung?

Marko Lindqvist wrote:

Any comment on this, Sveinung?

This was on my list of stuff that I either can do myself when I have a little limited time window for Freeciv or give as a "you are now good enough to do a real task in a real project" task to some friend I'm teaching C. I therefore didn't notice the attached patch when you assigned it to me.

Thank you for pointing it out, Marko. Sorry about the delay, Alexandro. I'll try to get it reviewed soon.

Overall impression is good.
Needs a rebase. (Sorry about the late review)
I suggest (but don't require) splitting out the different things you do in different patches. So moving is_effect_val_known() is in one patch, convertingn to switch is one, adding more meta knowledge another, etc. A good rule of thumb is that you start a sentence in the commit message with "Also" you should probably split the patch.

Hooh, splitting patches is not that fun as writing them... Well, let me try.

Alexandro Ignatiev wrote:

Hooh, splitting patches is not that fun as writing them...

Would the fact that you then will have more patches showing up under your name in OpenHub and GitHub help to motivate you?

Well, let me try.

You didn't upload anything yet, right? Just making sure that your new patch(es) hasn't been eaten by a bug.

Sveinung Kvilhaugsvik wrote:

You didn't upload anything yet, right? Just making sure that your new patch(es) hasn't been eaten by a bug.

Latest my patch on this topic was 6 months ago.

A correction to your current patch: Surviving tech reqs with the range "World" are visible to everyone. I added an explanation why in Feature #922369 I wished your clean up that changes stuff to use switch already had landed while I wrote it.

Any progress on this? We might go to code-freeze before 2.6.5 relatively soon (in a couple of weeks, not months)