Re: Regarding Outreachy project on Improving CR Dashboard

["Jesus M. Gonzalez-Barahona" <jgb@bitergia.com>]

On Wed, 2016-04-06 at 17:30 +0530, Priya wrote:
> Hello,
> 
> Thanks for your suggestions.
> I have made the appropriate changes as you had mentioned.
> It took a little time to change from python3 to python3.4 as perceval
> supports python3.4. I have updated the changes in my github. You can
> see my git repo [1]
> 
> [1]:https://github.com/priya299/Dashboard

Thanks a lot, Priya. Good work. Some preliminary comments, below.

* When runing the script on the xen-devel-2016-03 mbox, I seen an
exception raised:

------------
(perceval)jgb@expisito:~/src/outreachy/Dashboard/dashboard$ python3
createjson.py --mbox xen-devel-2016-03 --output new.json
Traceback (most recent call last):
  File "createjson.py", line 61, in <module>
    create_json(args.mbox,args.output)
  File "createjson.py", line 43, in create_json
    if key == k['Message-ID'].strip('<>'):
KeyError: 'Message-ID'
------------

Maybe some message is not having a Message-ID field? I suggest that you
capture this exception, print out the offending message, and go on with
the next one. You can use the Python logging package for printing out
this kind of information (you can see how to use it in the Perceval
package itself). But see below.

* Minor typo in the README:

Instead of 

eg: python3.4 createjson --mbox xen-devel-2016-03 --output new.json

it should be

eg: python3.4 createjson.py --mbox xen-devel-2016-03 --output new.json

* The files have no licensing info. If you agree, it could be GPLv3, as
is Perceval itself. For that, it would be enough that you mimic the
header in Perceval files in your Python files (of course, indicating
your authorship information).

* Which version of Perceval are you using? Some weeks ago, the format
of the dictionary produced by Perceval for each message changed. Now
the  actual fields of the message are in a data subdictionary. Please,
check that: the above exception with respect to the Message-ID key
could be because of this... Please, try to make it work with master
HEAD for Perceval (I don't expect any new major change in the next
days/weeks, and I'll try to warn you in case some happens).

* Could you please write at least one unit test for your code? You can
see examples of the testing schema we use in the tests directory in
Perceval, but we use vanilla unittest (the Python package for tests).
At this stage I don't need that you produce a whole set of tests, only
one or two to show that you know how to write unit tests, please.

Saludos,

	Jesus.

> Priya V
> Amrita University
> LinkedIn | GitHub | Bitbucket
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
-- 
Bitergia: http://bitergia.com
/me at Twitter: https://twitter.com/jgbarah


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel