Code review comment for lp:~cardinot/stellarium/MeteorShower

Revision history for this message
Marcos Cardinot (cardinot) wrote :

> > Hi Georg, thanks for your review! ;)
>
> Hi!
>
> Compilation with MSVC2013 works now.
>
> > > Somewhere in the config panel, can you please describe what's a
> > > real/generic/inactive radiant?
> >
> > Yeah, I will include a description on the about tab... but I think we should
> > rename them to something more intuitive...
> >
> > Real: it means that this data comes from a real IMO catalog - confirmed
> meteor
> > shower
> > Generic: it means that this radiant is usually active at this time of year,
> > but it was not confirmed from IMO.
> > Inactive: it means that this radiant is not active at this time of year.
> >
> > Maybe we should rename to...
> > Real -> Confirmed
> > Generic -> Unconfirmed
> >
> > ideas?
>
> Real->IMO confirmed
> Generic -> average data

Humm... the problem is that sometimes we (or even the user) could add a new shower, in which the data was confirmed by some other catalog - in this way, "IMO confirmed" would be too specific...

>
> Another thing: After Searching showers in te search panel, they are sorted by
> name. I would prefer sort-by-date. ... Oh that works even with a table header
> click, good!

Yeah, it's possible to sort the columns by clicking on the header =D
But I agree with you - I pushed a patch to always prefer sort-by-date...

>
> But if I want to sort now by ZHR (can you change the table heading to ZHR?),

But it's "ZHR" already - not?
http://bazaar.launchpad.net/~mcardinot/stellarium/MeteorShower/view/head:/plugins/MeteorShowers/src/gui/MSSearchDialog.cpp#L180

> I am not sure how sorting works for min-max numbers? It's a minor issue, but
> just in case it's a simple fix, decide for min/average/max as sort value?

good catch - thanks!
I believe that usually users would look for higher ZHRs, so I fixed it to always choose max...

« Back to merge proposal