n76 / lacells-creator

Script for generating a cell tower database for NetworkLocation from the microG project
GNU General Public License v3.0
45 stars 19 forks source link

Config file loading #14

Closed wvengen closed 9 years ago

wvengen commented 9 years ago

After a discussion in #13, this is the result. In addition to the proposal there, this also reads gen_lacells_conf for backward-compatibility (though the adb push configuration is not recognised).

n76 commented 9 years ago

Looks reasonable to me.

@IzzySoft please take a look and let us know what you think before I merge this request.

IzzySoft commented 9 years ago

@n76 I took a quick look last night, and am pretty much impressed :innocent: Didn't test it yet, though (found no time for that yet) – but so far, it looks great. No objections from my end. @wvengen even documented everything amazingly thorough, so even with the config being named a little different than expected ("creator" is not what I'd think of first – but he will have his reasons for chosing that name), all should be well.

wvengen commented 9 years ago

Thanks! I choose creator.cfg because I could imagine there might be other lacells-related tools in the future, which could use the same config directory (and perhaps share some). So ~/.config/lacells would be there for that. Then creator.cfg is a sensible name, and it makes sense to use the same name at other places. Not sure if the consideration I had is really perfect, but it's one angle that would work. I'm totally fine if another name is more sensible, feel free to open an issue or PR.

wvengen commented 9 years ago

I remember a note about trying to read misc/creator.cfg - that's the default and must be read (default file locations, or commands would fail).

IzzySoft commented 9 years ago

As for other lacell-related tools: not sure how @n76 thinks about it, but my first thought would be they'd share a common config file (e.g. lacells.cfg; instead of having multiple config files with two lines each). But without knowing what tools those might be, it's hard to tell which variant makes more sense. In that context, creator.cfg absolutely makes sense to me as being related to "create" stuff (db/csv files). Though generally speaking, all scripts might "create" something (statistics, maps, whatever).

But I agree: Let's leave it this way for now – as without having an idea about those other tools, changes make not much sense :stuck_out_tongue_winking_eye:

And yeah, that comment was from me, and visible for about 30s when I realized the /misc part and removed it. Sorry for any confusion that might have caused!