[chirp_devel] patch issue 1353 TH9800 basic support
2014-12-31 15:25 GMT-08:00 jml jml@jmlzone.com:
I know this is a small detail, but you need # in front of the issue number for it to be detected correctly.
You have some lines greater than 79 characters. These should be wrapped. https://www.python.org/dev/peps/pep-0008/#maximum-line-length
The patch is really funky and hard to read--it intermingles _upload() and old set_memory(). I'll have to import this before I can comment on much more.
Glad to see someone picking up support for this radio. There seems to be a lot of demand.
Tom KD7LXL
There is a large delta of change, we reworked the classes quite a bit so perhaps James can throw the new version in a pastebin for chirp-devs to review.
Sent from my iPhone
On Dec 31, 2014, at 6:50 PM, Tom Hayward tom@tomh.us wrote:
2014-12-31 15:25 GMT-08:00 jml jml@jmlzone.com:
I know this is a small detail, but you need # in front of the issue number for it to be detected correctly.
You have some lines greater than 79 characters. These should be wrapped. https://www.python.org/dev/peps/pep-0008/#maximum-line-length
The patch is really funky and hard to read--it intermingles _upload() and old set_memory(). I'll have to import this before I can comment on much more.
Glad to see someone picking up support for this radio. There seems to be a lot of demand.
Tom KD7LXL _______________________________________________ chirp_devel mailing list chirp_devel@intrepid.danplanet.com http://intrepid.danplanet.com/mailman/listinfo/chirp_devel Developer docs: http://chirp.danplanet.com/projects/chirp/wiki/Developers
Thanks for looking at it Tom, I accept the criticism as its my first cut and I had troubles. Do I need to do anything about the issue number and line length or will you take care of that? Happy new year.
James
On Dec 31, 2014, at 6:50 PM, Tom Hayward tom@tomh.us wrote:
2014-12-31 15:25 GMT-08:00 jml jml@jmlzone.com:
I know this is a small detail, but you need # in front of the issue number for it to be detected correctly.
You have some lines greater than 79 characters. These should be wrapped. https://www.python.org/dev/peps/pep-0008/#maximum-line-length
The patch is really funky and hard to read--it intermingles _upload() and old set_memory(). I'll have to import this before I can comment on much more.
Glad to see someone picking up support for this radio. There seems to be a lot of demand.
Tom KD7LXL _______________________________________________ chirp_devel mailing list chirp_devel@intrepid.danplanet.com http://intrepid.danplanet.com/mailman/listinfo/chirp_devel Developer docs: http://chirp.danplanet.com/projects/chirp/wiki/Developers
On Dec 31, 2014 9:14 PM, "jml" jml@jmlzone.com wrote:
Thanks for looking at it Tom, I accept the criticism as its my first cut and I had troubles. Do I
need to do anything about the issue number and line length or will you take care of that?
Happy new year.
I'm just observing. Only Dan has commit privileges and he'll want those things fixed before committing.
Tom KD7LXL
I accept the criticism as its my first cut and I had troubles.
Definitely don't take offense -- code review is part of open source, and it's a good thing :)
Do I need to do anything about the issue number and line length or will you take care of that? Happy new year.
I'm just observing. Only Dan has commit privileges and he'll want those things fixed before committing.
Yep, my scripts won't even allow it into the tree without a proper commit message that references the issue number with a # sign in front. With mq, it's really easy to amend a patch to make a few changes and resubmit.
Thanks!
--Dan
OK I have fixed the line length, and and subject line hopefully it goes this time.
On Jan 1, 2015, at 1:56 PM, Dan Smith dsmith@danplanet.com wrote:
I accept the criticism as its my first cut and I had troubles.
Definitely don't take offense -- code review is part of open source, and it's a good thing :)
Do I need to do anything about the issue number and line length or will you take care of that? Happy new year.
I'm just observing. Only Dan has commit privileges and he'll want those things fixed before committing.
Yep, my scripts won't even allow it into the tree without a proper commit message that references the issue number with a # sign in front. With mq, it's really easy to amend a patch to make a few changes and resubmit.
Thanks!
--Dan
chirp_devel mailing list chirp_devel@intrepid.danplanet.com http://intrepid.danplanet.com/mailman/listinfo/chirp_devel Developer docs: http://chirp.danplanet.com/projects/chirp/wiki/Developers
participants (4)
-
Dan Smith -
Jens -
jml -
Tom Hayward