Change Details

{icon calendar} | Friday, 17th June 2016 - 15:30 UTC {icon user} | jayvdb, darthbhyrava (//legoktm could not make it due to personal reasons//) **Skype** **Description**: Fourth weekly project meeting. **Start**: 15.30 UTC **End**: 16.30 UTC *** === Agenda - Review patch for T135409 (will be up soon) - Review the following code for T135411 and get it patch ready: - Prototype for flow_thanks.py (should it be merged with thanks.py?) P3256 (site.py code in P3257) - Design of unit tests for flow based thanks. ( same questions as above for tests) - Plan till the 20th June deadline. - Discussion on MVP and objectives for the 27th Mid-Term deadline. *** === Minutes of the Meeting . ===== Who fixes the bug? - T137628 is an unresolved pywikibot issue, responsible for `tox-jessie` failing for all pywikibot patches of late. Due to this, darthbhyrava's patch [[ https://gerrit.wikimedia.org/r/#/c/294901/ | 294901 ]] is failing at `tox-jessie`, too. - darthbhyrava needs a `+2` from `jenkins-bot` for patches to tasks T135409 and T135411 to pass mid term evaluations for his GSoC project. And if there are bugs in pywikibot which keep the patches from receiving a +2, it is his responsibility as part of the pywikibot team to resolve them, as much it is as anyone else's, according to jayvdb. - However, not a priority now - priority is to get flow based thanks working. jayvdb is looking at how to fix the bug. This will be discussed further in the next meet, depending on the progress of the MVP, and if jayvdb can't fix it by then. ===== Brief summary of what darthbhyrava's been upto. - Thanks for normal revisions: T135409. See patch [[ https://gerrit.wikimedia.org/r/#/c/294901/ | here ]]. - `scripts/thanks.py`: Extracting user from the revision using recentchanges to check if thanks is enabled for that user. What if the revision is not in recentchanges? Then User.thanks_enabled is assumed to be True. If User.thanks_enabled value is true, call a thank_revision() method in site.py to make an API call for thanking the revision. // Incorrect approach to getting user for thanks_enabled, implement the approach suggested by legoktm in P3258#15427 by adding a method to site.py and calling it in the code. // - `tests/flow_thanks_tests.py`: Testing on test:test. I am getting the revision of the most recent revision, then checking log id of most recent thanks. Then I thank the revision, and check the latest thanks log id, again. They must be unequal. Added a note to recent changes stating that this test relies on activity in recentchanges. - `User.thanks_enabled` in page.py: Adding in the method with the caveat that it did not accurately determine if user has thanks enabled, and the link to related discussions on Phabricator. - `thanks_revision` method added to site.py for API Calls for thanks. - @Legoktm reviewed the changes before the patch was pushed, please see P3258, P3249 and P3243. - Thanks for flow comments: T135411. Patch will be up soon. - `flow_thanks.py`: Is very similar to thanks.py. However, instead of thanking a given revision ID, it thanks a post_id for a Flow post. It also doesn't check for user.thanks_enabled, yet. It calls a thanks_flowpost() method in site.py to make the flowthank API call. - `flow_thanks_tests`: Only a design is in place, no code yet. The plan is to extract post_ids from a Flow Page, thank one, and then compare log for thanks before and after thanking, similar to thanks_tests.py. The next part is implementing this - how to get a Board, then a Topic, then a Flow post, and then how to extract the id of a post. ===== Comments on thanks.py and other Subtask I code. - The code looks good. The source parameter in the API call is a good idea. - jayvdb and legoktm will look at the related [patch](https://gerrit.wikimedia.org/r/#/c/294901/) later. - In thanks_tests, only one revision is being extracted. What if this revision's user can't be thanked? - User_id should not be extracted from recentchanges, use the approach mentioned by legoktm, add a new method to site.py for the necessary API call. - The thanks script works well, and unit tests can't be easily made to fail. ===== Comments on flow_thanks.py and other Subtask II code. - Remove the 'format':'json' key-value pair in site.py, it is there by default. - Query: Is it necessary to have both the extensions for the site.py code for flow_thanks.py? - In api help, it mentions Source:Thanks for action=flowthank. So thanks is definitely required. - Task: get the Thank extension code, and look at the FlowThank API module ( ApiFlowThank.php ), and look for some sign this module does not load unless Flow is also enabled. //We finally find that there is no hard requiremnt for Flow, but Echo is required. However, since Thanks has a hard requirement for Echo, that is not an issue.// Now look at documentation, it states [here](https://www.mediawiki.org/wiki/Extension:Thanks#Usage) that the user will need flow. Now this thank action will fail if Flow is not installed. Hence Flow is also required, but this needs confirmation. - Next task, and this could be difficult, is to check if site.py ensures that two need_extensions can co-exist. No other method seems to have the need for two extensions in site.py. //Check on local wiki, for example.// - Query: Can we include 'print' statements in scripts? They don't seem to be in any other scripts. //No, but you shouldn't. But you can use pywikibot.output instead.// - For flow_thanks_tests, Alex was setting up his unit tests in flow_tests, should this be similar? //That was a container class, and it's darthbhyrava's responsibility to figure it out.// - Approach for the flow unit tests sounds good, but in order to be reproducible, the post needs to be the most recent one, on the most recent topic. - Flow posts can //also// be anonymous, so it needs to be accounted for while thanking a flow post, if it is doable. If it needs a lot of queries, lots of hops, then just add a caveat and ignore this instead of adding in silly code. - Also, do not merge `flow_thanks.py` and `thanks.py` now. Submit them separately. ===== jayvdb on the jenkins problem: If jenkins is not fixed by the 27th, and if darthbhyrava does not have his patches `+2`ed by then, then he fails. If jayvdb can't resolve the issue himself, then darthbhyrava must fix it himself or get others to, as he's paid and others are not, and since he has a deadline, and others don't. The aforementioned issue has been there for three weeks, and no one has fixed it, yet. *** === Before Next Meet [ ] Update Weekly Reports [x] Update Blog Post - [[ http://bhyrava.me/code/gsoc/wikimedia/2016/06/17/GSoC6/ | Post on Subtask II ]] [x] Submit first patchset for T135409: [[ https://gerrit.wikimedia.org/r/#/c/294901 | 294901 ]] [ ] Submit first patchset for T135411: [x] Implement P3258#15427 in `thanks.py` for getting user. [x] Account for User.thanks_enabled in `thanks_tests.py` before thanking. - Added 'showAnon=False' as param in recentchanges. Will change to similar method in thanks.py if deemed necessary. [ ] Remove 'format':'json' from flow code. [ ] Check if site.py ensures that two need_extensions can co-exist. [ ] Remove 'print' statements in all code. [ ] For `flow_thanks_tests.py`, extract most recent post under most recent topic. [ ] Account for User.thanks_enabled in flow code if it is simple and doable, else add note and ignore.

{icon calendar} | Friday, 17th June 2016 - 15:30 UTC {icon user} | jayvdb, darthbhyrava (//legoktm could not make it due to personal reasons//) **Skype** **Description**: Fourth weekly project meeting. **Start**: 15.30 UTC **End**: 16.30 UTC *** === Agenda - Review patch for T135409 (will be up soon) - Review the following code for T135411 and get it patch ready: - Prototype for flow_thanks.py (should it be merged with thanks.py?) P3256 (site.py code in P3257) - Design of unit tests for flow based thanks. ( same questions as above for tests) - Plan till the 20th June deadline. - Discussion on MVP and objectives for the 27th Mid-Term deadline. *** === Minutes of the Meeting . ===== Who fixes the bug? - T137628 is an unresolved pywikibot issue, responsible for `tox-jessie` failing for all pywikibot patches of late. Due to this, darthbhyrava's patch [[ https://gerrit.wikimedia.org/r/#/c/294901/ | 294901 ]] is failing at `tox-jessie`, too. - darthbhyrava needs a `+2` from `jenkins-bot` for patches to tasks T135409 and T135411 to pass mid term evaluations for his GSoC project. And if there are bugs in pywikibot which keep the patches from receiving a +2, it is his responsibility as part of the pywikibot team to resolve them, as much it is as anyone else's, according to jayvdb. - However, not a priority now - priority is to get flow based thanks working. jayvdb is looking at how to fix the bug. This will be discussed further in the next meet, depending on the progress of the MVP, and if jayvdb can't fix it by then. ===== Brief summary of what darthbhyrava's been upto. - Thanks for normal revisions: T135409. See patch [[ https://gerrit.wikimedia.org/r/#/c/294901/ | here ]]. - `scripts/thanks.py`: Extracting user from the revision using recentchanges to check if thanks is enabled for that user. What if the revision is not in recentchanges? Then User.thanks_enabled is assumed to be True. If User.thanks_enabled value is true, call a thank_revision() method in site.py to make an API call for thanking the revision. // Incorrect approach to getting user for thanks_enabled, implement the approach suggested by legoktm in P3258#15427 by adding a method to site.py and calling it in the code. // - `tests/flow_thanks_tests.py`: Testing on test:test. I am getting the revision of the most recent revision, then checking log id of most recent thanks. Then I thank the revision, and check the latest thanks log id, again. They must be unequal. Added a note to recent changes stating that this test relies on activity in recentchanges. - `User.thanks_enabled` in page.py: Adding in the method with the caveat that it did not accurately determine if user has thanks enabled, and the link to related discussions on Phabricator. - `thanks_revision` method added to site.py for API Calls for thanks. - @Legoktm reviewed the changes before the patch was pushed, please see P3258, P3249 and P3243. - Thanks for flow comments: T135411. Patch will be up soon. - `flow_thanks.py`: Is very similar to thanks.py. However, instead of thanking a given revision ID, it thanks a post_id for a Flow post. It also doesn't check for user.thanks_enabled, yet. It calls a thanks_flowpost() method in site.py to make the flowthank API call. - `flow_thanks_tests`: Only a design is in place, no code yet. The plan is to extract post_ids from a Flow Page, thank one, and then compare log for thanks before and after thanking, similar to thanks_tests.py. The next part is implementing this - how to get a Board, then a Topic, then a Flow post, and then how to extract the id of a post. ===== Comments on thanks.py and other Subtask I code. - The code looks good. The source parameter in the API call is a good idea. - jayvdb and legoktm will look at the related [patch](https://gerrit.wikimedia.org/r/#/c/294901/) later. - In thanks_tests, only one revision is being extracted. What if this revision's user can't be thanked? - User_id should not be extracted from recentchanges, use the approach mentioned by legoktm, add a new method to site.py for the necessary API call. - The thanks script works well, and unit tests can't be easily made to fail. ===== Comments on flow_thanks.py and other Subtask II code. - Remove the 'format':'json' key-value pair in site.py, it is there by default. - Query: Is it necessary to have both the extensions for the site.py code for flow_thanks.py? - In api help, it mentions Source:Thanks for action=flowthank. So thanks is definitely required. - Task: get the Thank extension code, and look at the FlowThank API module ( ApiFlowThank.php ), and look for some sign this module does not load unless Flow is also enabled. //We finally find that there is no hard requiremnt for Flow, but Echo is required. However, since Thanks has a hard requirement for Echo, that is not an issue.// Now look at documentation, it states [here](https://www.mediawiki.org/wiki/Extension:Thanks#Usage) that the user will need flow. Now this thank action will fail if Flow is not installed. Hence Flow is also required, but this needs confirmation. - Next task, and this could be difficult, is to check if site.py ensures that two need_extensions can co-exist. No other method seems to have the need for two extensions in site.py. //Check on local wiki, for example.// - Query: Can we include 'print' statements in scripts? They don't seem to be in any other scripts. //No, but you shouldn't. But you can use pywikibot.output instead.// - For flow_thanks_tests, Alex was setting up his unit tests in flow_tests, should this be similar? //That was a container class, and it's darthbhyrava's responsibility to figure it out.// - Approach for the flow unit tests sounds good, but in order to be reproducible, the post needs to be the most recent one, on the most recent topic. - Flow posts can //also// be anonymous, so it needs to be accounted for while thanking a flow post, if it is doable. If it needs a lot of queries, lots of hops, then just add a caveat and ignore this instead of adding in silly code. - Also, do not merge `flow_thanks.py` and `thanks.py` now. Submit them separately. ===== jayvdb on the jenkins problem: If jenkins is not fixed by the 27th, and if darthbhyrava does not have his patches `+2`ed by then, then he fails. If jayvdb can't resolve the issue himself, then darthbhyrava must fix it himself or get others to, as he's paid and others are not, and since he has a deadline, and others don't. The aforementioned issue has been there for three weeks, and no one has fixed it, yet. *** === Before Next Meet [ ] Update Weekly Reports - partially, update more info. [x] Update Blog Post - [[ http://bhyrava.me/code/gsoc/wikimedia/2016/06/17/GSoC6/ | Post on Subtask II ]] [x] Submit first patchset for T135409: [[ https://gerrit.wikimedia.org/r/#/c/294901 | 294901 ]] [x] Submit first patchset for T135411: [[ https://gerrit.wikimedia.org/r/#/c/295132/ | 295132 ]] [x] Implement P3258#15427 in `thanks.py` for getting user. [x] Account for User.thanks_enabled in `thanks_tests.py` before thanking. - Added 'showAnon=False' as param in recentchanges. Will change to similar method in thanks.py if deemed necessary. [x] Remove 'format':'json' from flow code. [x] Check if site.py ensures that two need_extensions can co-exist. - Line 1343 in site,py defines need_extension, which doesn't seem to indicate clash with other decorators. [x] Remove 'print' statements in all code. [x] For `flow_thanks_tests.py`, extract most recent post under most recent topic. [x] Account for User.thanks_enabled in flow code if it is simple and doable, else add note and ignore. - No simple doable way of extracting user from flow post id, hence this is being skipped with the added caveat.

{icon calendar} | Friday, 17th June 2016 - 15:30 UTC {icon user} | jayvdb, darthbhyrava (//legoktm could not make it due to personal reasons//) **Skype** **Description**: Fourth weekly project meeting. **Start**: 15.30 UTC **End**: 16.30 UTC *** === Agenda - Review patch for T135409 (will be up soon) - Review the following code for T135411 and get it patch ready: - Prototype for flow_thanks.py (should it be merged with thanks.py?) P3256 (site.py code in P3257) - Design of unit tests for flow based thanks. ( same questions as above for tests) - Plan till the 20th June deadline. - Discussion on MVP and objectives for the 27th Mid-Term deadline. *** === Minutes of the Meeting . ===== Who fixes the bug? - T137628 is an unresolved pywikibot issue, responsible for `tox-jessie` failing for all pywikibot patches of late. Due to this, darthbhyrava's patch [[ https://gerrit.wikimedia.org/r/#/c/294901/ | 294901 ]] is failing at `tox-jessie`, too. - darthbhyrava needs a `+2` from `jenkins-bot` for patches to tasks T135409 and T135411 to pass mid term evaluations for his GSoC project. And if there are bugs in pywikibot which keep the patches from receiving a +2, it is his responsibility as part of the pywikibot team to resolve them, as much it is as anyone else's, according to jayvdb. - However, not a priority now - priority is to get flow based thanks working. jayvdb is looking at how to fix the bug. This will be discussed further in the next meet, depending on the progress of the MVP, and if jayvdb can't fix it by then. ===== Brief summary of what darthbhyrava's been upto. - Thanks for normal revisions: T135409. See patch [[ https://gerrit.wikimedia.org/r/#/c/294901/ | here ]]. - `scripts/thanks.py`: Extracting user from the revision using recentchanges to check if thanks is enabled for that user. What if the revision is not in recentchanges? Then User.thanks_enabled is assumed to be True. If User.thanks_enabled value is true, call a thank_revision() method in site.py to make an API call for thanking the revision. // Incorrect approach to getting user for thanks_enabled, implement the approach suggested by legoktm in P3258#15427 by adding a method to site.py and calling it in the code. // - `tests/flow_thanks_tests.py`: Testing on test:test. I am getting the revision of the most recent revision, then checking log id of most recent thanks. Then I thank the revision, and check the latest thanks log id, again. They must be unequal. Added a note to recent changes stating that this test relies on activity in recentchanges. - `User.thanks_enabled` in page.py: Adding in the method with the caveat that it did not accurately determine if user has thanks enabled, and the link to related discussions on Phabricator. - `thanks_revision` method added to site.py for API Calls for thanks. - @Legoktm reviewed the changes before the patch was pushed, please see P3258, P3249 and P3243. - Thanks for flow comments: T135411. Patch will be up soon. - `flow_thanks.py`: Is very similar to thanks.py. However, instead of thanking a given revision ID, it thanks a post_id for a Flow post. It also doesn't check for user.thanks_enabled, yet. It calls a thanks_flowpost() method in site.py to make the flowthank API call. - `flow_thanks_tests`: Only a design is in place, no code yet. The plan is to extract post_ids from a Flow Page, thank one, and then compare log for thanks before and after thanking, similar to thanks_tests.py. The next part is implementing this - how to get a Board, then a Topic, then a Flow post, and then how to extract the id of a post. ===== Comments on thanks.py and other Subtask I code. - The code looks good. The source parameter in the API call is a good idea. - jayvdb and legoktm will look at the related [patch](https://gerrit.wikimedia.org/r/#/c/294901/) later. - In thanks_tests, only one revision is being extracted. What if this revision's user can't be thanked? - User_id should not be extracted from recentchanges, use the approach mentioned by legoktm, add a new method to site.py for the necessary API call. - The thanks script works well, and unit tests can't be easily made to fail. ===== Comments on flow_thanks.py and other Subtask II code. - Remove the 'format':'json' key-value pair in site.py, it is there by default. - Query: Is it necessary to have both the extensions for the site.py code for flow_thanks.py? - In api help, it mentions Source:Thanks for action=flowthank. So thanks is definitely required. - Task: get the Thank extension code, and look at the FlowThank API module ( ApiFlowThank.php ), and look for some sign this module does not load unless Flow is also enabled. //We finally find that there is no hard requiremnt for Flow, but Echo is required. However, since Thanks has a hard requirement for Echo, that is not an issue.// Now look at documentation, it states [here](https://www.mediawiki.org/wiki/Extension:Thanks#Usage) that the user will need flow. Now this thank action will fail if Flow is not installed. Hence Flow is also required, but this needs confirmation. - Next task, and this could be difficult, is to check if site.py ensures that two need_extensions can co-exist. No other method seems to have the need for two extensions in site.py. //Check on local wiki, for example.// - Query: Can we include 'print' statements in scripts? They don't seem to be in any other scripts. //No, but you shouldn't. But you can use pywikibot.output instead.// - For flow_thanks_tests, Alex was setting up his unit tests in flow_tests, should this be similar? //That was a container class, and it's darthbhyrava's responsibility to figure it out.// - Approach for the flow unit tests sounds good, but in order to be reproducible, the post needs to be the most recent one, on the most recent topic. - Flow posts can //also// be anonymous, so it needs to be accounted for while thanking a flow post, if it is doable. If it needs a lot of queries, lots of hops, then just add a caveat and ignore this instead of adding in silly code. - Also, do not merge `flow_thanks.py` and `thanks.py` now. Submit them separately. ===== jayvdb on the jenkins problem: If jenkins is not fixed by the 27th, and if darthbhyrava does not have his patches `+2`ed by then, then he fails. If jayvdb can't resolve the issue himself, then darthbhyrava must fix it himself or get others to, as he's paid and others are not, and since he has a deadline, and others don't. The aforementioned issue has been there for three weeks, and no one has fixed it, yet. *** === Before Next Meet [ ] Update Weekly Reports - partially, update more info. [x] Update Blog Post - [[ http://bhyrava.me/code/gsoc/wikimedia/2016/06/17/GSoC6/ | Post on Subtask II ]] [x] Submit first patchset for T135409: [[ https://gerrit.wikimedia.org/r/#/c/294901 | 294901 ]] [ x] Submit first patchset for T135411: [[ https://gerrit.wikimedia.org/r/#/c/295132/ | 295132 ]] [x] Implement P3258#15427 in `thanks.py` for getting user. [x] Account for User.thanks_enabled in `thanks_tests.py` before thanking. - Added 'showAnon=False' as param in recentchanges. Will change to similar method in thanks.py if deemed necessary. [ x] Remove 'format':'json' from flow code. [ x] Check if site.py ensures that two need_extensions can co-exist. [ ] Remove 'print' statements in all code - Line 1343 in site,py defines need_extension, which doesn't seem to indicate clash with other decorators. [ ] For `flow_thanks_tests.py`, extract most recent post under most recent topicx] Remove 'print' statements in all code. [x] For `flow_thanks_tests.py`, extract most recent post under most recent topic. [x] Account for User.thanks_enabled in flow code if it is simple and doable, else add note and ignore. - No simple doable way of extracting user from flow post id, else add note and ignorehence this is being skipped with the added caveat.