| Friday, 18th June 2016 - 13:00 UTC (21:00 SGT)
| jayvdb, darthbhyrava

Skype
Description: A brief weekly meeting to discuss MVP and Mid-Term Evaluation.

Agenda

• Review patch for T135409 : 294901.
• Review patch for T135411: 295132
  • Paste for flow_thanks_tests code: P3270
• Review weekly reports: T133667
• Discuss T137628, if it has not yet been resolved.
• Discuss remaining Mid-Term Evaluation objectives for the 27th Deadline, if any.

Minutes of the Meeting

.

Brief summary of progress, with rolling feedback.

• Subtask I : Patchset 2
  • In thanks.py, shifted from using recentchanges to extract user to a get_revision method in site.py. Implemented a get_revision(revid) method instead of a get_user(revid) directly, since the user can be extracted from the former, and the latter is too specific.To accomodate for this, I had to use a recursive get_value() function which would iterate over the nested dict to get the user details.
    • No, don't go for this approach to implementation. The revision is always in a fixed location, so no need for recursion to search for the location. Use the Revision Class from page.py, and look at the code that uses the Revision class, as this is a functionality that others have already used.
  • In thanks_tests.py, I used a hack - a parameter showAnon=False in recentchanges, to get things working for now, while the approach for extracting user is figured out.
    • No, this a hack, please ensure that the decision of determining thanks is encapsulated within User.thanks_enabled.
    • self.asserNotEqual in thanks_tests asserts that there are two different log entries, and not that we've found one after the last log entry. This stems from the total=1 param. Use, say total=50, and then check within these log entries for the one we need. To the best of your ability, you have to assert that a logevent was created for the thanks actions.

• Subtask II : Patchset 2
  • In flow_thanks_tests.py, removed the Exception handling for the bug (needs to be filed), by restricting the iterations to first 10.
    • Isn't sort_by=updated param throwing up errors? Look into it.
    • What if the exception for the bug is updated, again? Then it should cause the same error. Firstly, make a task for the bug. Secondly, as a workaround, get two topics, and if the most recent one is for this topic, go for the next one.
    • We need only the recently updated post, so we don't need islice(), since it is creating a generator, which leaves an operation incomplete in progress. We want the operation to complete. Use ordinary slice instead, since you're using the same name, too, there will not be another generator, and the operation will be complete by the end of the line.
    • We require only one post_id, then why do you need a list? The name post_ids seems to indicate you are using multiple posts. Use better names, say post_tobethanked.
    • Change the assertion approach here, too. An operation on the wiki takes upto half an hour, you need to be utilising that time accurately and more efficiently. Some developers go through unit test logs from six months to check when errors crop up. The assertions must be as meaningful, precise and strict as possible.
• Other Work:
  • Checking up on @need_extension: Looked at need_extension() in site.py to check for code which might cause problems if there are two required. Found none. Also tried the thank action with and without the Flow extension on local wiki.
    • You can use python debugger pdb to do the same check as in site.py, and get a more definite and certain way of doing things.
  • Blog Post on Subtask II up here. Also, previous post has been updated.
    • An idea for a blog post is the Flow vs Thanks experiment, both the PHP and Python sides of it, using the python debugger. Once this is done, add to the documentation regarding interactions between both.
  • Extracting a user for the flow code. There were no simple approaches to getting the user extracted, thought there probably are approaches involving multiple calls which can achieve the same.
    • Write your own code to get the User. Check the PHP Api Calls, they will help.

More reviews will come in once the patches have been submitted. Nothing major since the deadline is close, but many little changes will be suggested to improve the quality of the code. Also, there must be more tests to check for different kinds of errors, and tests for site.py methods.

Before Next Meet

Subtask I

• Use the Revision class instead of a recursive function to get revision details in thanks.py.
• Remove the hack and let User.thanks_enabled do the job in thanks_tests.py.
• Improve the assertion in unit test scripts to make it more precise and meaningful.
• Shift to variable revid consistently throughout.
• Add source as a user customisable parameter to def get_revision() in site.py
• Shift the above method in site.py next to the other revision methods.
• Shift thanks.py to maintenance folder.
• Adhere to coding linting rules.
• Remove u"" since we're using unicode literals.
• thank_rev should take APISite as a parameter.
• If user.thanks_enabled=False, then what?
• If user doesn't specify arguments, then what?
• Use self.assertRaises to check for exceptions thrown for bad data. Account for as many as possible.
• Change name of unit test class to a non-abbreviated one.
• Improve implementation of getting Site object.
• Add tests for new site.py method. What class, what to assert?
• Add write=True to thanks_tests as per tests/README Tests fail, ask me to add PYWIKIBOT2_TEST_WRITE=1
• Make direct calls to page.py and site.py for unit test. On hold - what are we testing, then?

Subtask II

• Look into and document all errors caused by sort_by='updated' param in flow_thanks_tests.py.
• Get a suitable workaround for the bug in flow_thanks_tests.py, and document the bug.
• Improve the assertion in unit test scripts to make it more precise and meaningful.
• Use ordinary slice instead of islice()
• Add tests for new site.py method.
• Shift flow_thanks.py to maintenance folder.
• Adhere to coding linting rules.
• Remove u"" since we're using unicode literals.
• thank_flowpost should take APISite as a parameter.
• If user.thanks_enabled=False, then what?
• Improve implementation of getting Site object.
• If user doesn't specify arguments, then what?
• Use self.assertRaises to check for exceptions thrown for bad data. Account for as many as possible.
• Remove the list post_ids[] and rename it appropriately.
• Add tests for new site.py method.
• Add tests for border cases.

Other

• Create the task for the possible Bug in flow code. jayvdb did the honours in T138306
• Use pdb to perform the Flow/Thanks interaction comparisons.
• Update weekly reports.
• Write a blog post regarding the comparisons, and document your process and results there.
• Update documentation regarding the results of this study.