Code review comment for lp:~jlcanales-gasco/stellarium/satellite-plugin

Revision history for this message
Bogdan Marinov (daggerstab) wrote :

I skimmed the code and I immediately saw a problem that needs to be fixed before proceeding with a merge.

It seems that you've used an old version of the source as the base for your modifications. As a result, they overwrite the changes I made to fix bug #616627:
https://bugs.launchpad.net/stellarium/+bug/616627

Here are the relevant revisions:
http://bazaar.launchpad.net/~stellarium/stellarium/trunk/revision/4756
http://bazaar.launchpad.net/~stellarium/stellarium/trunk/revision/4757 (this is just an update of the TLEs)
http://bazaar.launchpad.net/~stellarium/stellarium/trunk/revision/4758

The links are for information only, just use the most recent code.

After you merge, you can update the TLEs again, just make sure that no satellites have been dropped off the lists at celestrak.com or renamed (again :(). My patch provides a log message that reports how many satellites have been updated out of the total number.

review: Needs Fixing

« Back to merge proposal