The proposal to merge lp:~alisonken1/openlp/pjlink2-f into lp:openlp has been
updated.
Status: Needs review => Merged
For more details, see:
https://code.launchpad.net/~alisonken1/openlp/pjlink2-f/+merge/327801
--
Your team OpenLP Core is subscribed to branch lp:openlp.
___
Review: Approve
--
https://code.launchpad.net/~alisonken1/openlp/pjlink2-f/+merge/327801
Your team OpenLP Core is subscribed to branch lp:openlp.
___
Mailing list: https://launchpad.net/~openlp-core
Post to : openlp-core@lists.launchpad.net
Unsubs
Review: Approve
--
https://code.launchpad.net/~alisonken1/openlp/pjlink2-f/+merge/327801
Your team OpenLP Core is subscribed to branch lp:openlp.
___
Mailing list: https://launchpad.net/~openlp-core
Post to : openlp-core@lists.launchpad.net
Unsubs
The proposal to merge lp:~alisonken1/openlp/pjlink2-f into lp:openlp has been
updated.
Status: Needs review => Superseded
For more details, see:
https://code.launchpad.net/~alisonken1/openlp/pjlink2-f/+merge/327290
--
Your team OpenLP Core is subscribed to branch lp:openlp.
___
Ken Roberts has proposed merging lp:~alisonken1/openlp/pjlink2-f into lp:openlp.
Commit message:
PJLink class 2 update F
Requested reviews:
Tomas Groth (tomasgroth)
Raoul Snyman (raoul-snyman)
Tim Bentley (trb143)
For more details, see:
https://code.launchpad.net/~alisonken1/openlp/pjlink2
The functions defined are laying the groundwork. The functions will be
called from the "self.pjlink_functions" defined at line 155.
at this point, I'm working on the changes incrementally - plus, trying
to do some refactoring at the same time. Since the update to PJLink
Class 2 is rather large, I
Review: Needs Fixing
See inline comments.
Please fix the commit comments to reflect the changes as they refer to #noqa
which has been removed. (There may be more).
Diff comments:
>
> === modified file 'openlp/core/lib/projector/pjlink1.py'
> --- openlp/core/lib/projector/pjlink1.py 2017-
Review: Approve
--
https://code.launchpad.net/~alisonken1/openlp/pjlink2-f/+merge/327290
Your team OpenLP Core is subscribed to branch lp:openlp.
___
Mailing list: https://launchpad.net/~openlp-core
Post to : openlp-core@lists.launchpad.net
Unsubs
Ken Roberts has proposed merging lp:~alisonken1/openlp/pjlink2-f into lp:openlp.
Commit message:
PJLink class 2 update F
Requested reviews:
Tim Bentley (trb143)
Raoul Snyman (raoul-snyman)
Tomas Groth (tomasgroth)
For more details, see:
https://code.launchpad.net/~alisonken1/openlp/pjlink2
The proposal to merge lp:~alisonken1/openlp/pjlink2-f into lp:openlp has been
updated.
Status: Needs review => Superseded
For more details, see:
https://code.launchpad.net/~alisonken1/openlp/pjlink2-f/+merge/327052
--
Your team OpenLP Core is subscribed to branch lp:openlp.
___
oops - thought I removed those two lines.
I also remember running jenkins, but guess I forgot to delete the
previous run from the request.
On Wed, Jul 12, 2017 at 12:30 AM, Tomas Groth wrote:
> Review: Needs Fixing
>
> You left some commented code in, see the inline comment. Also please post the
Review: Needs Fixing
You left some commented code in, see the inline comment. Also please post the
jenkins test result for the latest revision.
Otherwise it looks good to me.
Diff comments:
>
> === modified file 'openlp/core/lib/projector/pjlink1.py'
> --- openlp/core/lib/projector/pjlink1.py
Ken Roberts has proposed merging lp:~alisonken1/openlp/pjlink2-f into lp:openlp.
Commit message:
PJLink class 2 update F
Requested reviews:
Tim Bentley (trb143)
Tomas Groth (tomasgroth)
Raoul Snyman (raoul-snyman)
For more details, see:
https://code.launchpad.net/~alisonken1/openlp/pjlink2
The proposal to merge lp:~alisonken1/openlp/pjlink2-f into lp:openlp has been
updated.
Status: Needs review => Superseded
For more details, see:
https://code.launchpad.net/~alisonken1/openlp/pjlink2-f/+merge/326493
--
Your team OpenLP Core is subscribed to branch lp:openlp.
___
Review: Needs Fixing
Some comments inline.
I don't agree with suppressing flake8's warnings, either inline or in a config
file, especially the warnings you've suppressed. With our poor test coverage at
the moment, we'll find it hard to catch errors due to these mistakes until they
end up in ch
Review: Approve
--
https://code.launchpad.net/~alisonken1/openlp/pjlink2-f/+merge/326493
Your team OpenLP Core is subscribed to branch lp:openlp.
___
Mailing list: https://launchpad.net/~openlp-core
Post to : openlp-core@lists.launchpad.net
Unsubs
Ken Roberts has proposed merging lp:~alisonken1/openlp/pjlink2-f into lp:openlp.
Commit message:
PJLink class 2 update F
Requested reviews:
Tim Bentley (trb143)
Tomas Groth (tomasgroth)
For more details, see:
https://code.launchpad.net/~alisonken1/openlp/pjlink2-f/+merge/326493
-- Renamed t
The proposal to merge lp:~alisonken1/openlp/pjlink2-f into lp:openlp has been
updated.
Status: Needs review => Superseded
For more details, see:
https://code.launchpad.net/~alisonken1/openlp/pjlink2-f/+merge/326492
--
Your team OpenLP Core is subscribed to branch lp:openlp.
___
Ken Roberts has proposed merging lp:~alisonken1/openlp/pjlink2-f into lp:openlp.
Commit message:
PJLink class 2 update F
Requested reviews:
Tomas Groth (tomasgroth)
Tim Bentley (trb143)
For more details, see:
https://code.launchpad.net/~alisonken1/openlp/pjlink2-f/+merge/326492
-- Renamed t
The proposal to merge lp:~alisonken1/openlp/pjlink2-f into lp:openlp has been
updated.
Status: Needs review => Superseded
For more details, see:
https://code.launchpad.net/~alisonken1/openlp/pjlink2-f/+merge/326265
--
Your team OpenLP Core is subscribed to branch lp:openlp.
___
Hmm - underscore is still tagged as 'assigned but not used' - guess we'll have
to deal with it.
--
https://code.launchpad.net/~alisonken1/openlp/pjlink2-f/+merge/326265
Your team OpenLP Core is subscribed to branch lp:openlp.
___
Mailing list: https://
Hmm - forgot about underscore. Will cleanup and resubmit (probably Wed. since
I'll have time off then).
--
https://code.launchpad.net/~alisonken1/openlp/pjlink2-f/+merge/326265
Your team OpenLP Core is subscribed to branch lp:openlp.
___
Mailing list:
Review: Needs Fixing
You can use _ for unused variables.
Not convinced with the #noqa as we are not using Flake8 generally (at present)
There may be the need for Phil to do a cleanup patch to remove them in a years
time.
Diff comments:
>
> === modified file 'openlp/core/lib/projector/pjlink1.p
I can agree about the possible future imports - just means I need to put them
in a note somewhere so I can remember what I may need to use later.
As far as # noqa - For some warnings/errors it makes sense to put it in the
setup.cfg. And normally I would want to find out if I'm creating variables
Review: Needs Fixing
In general it looks good, but there are 2 things I'm not too happy about, the
use of "# noqa" and the "possible future imports".
While I think using pep8/pycodestyle, flake8 and pylint is great, I'm not too
happy about us having to "pollute" the source code with hints of whe
Ken Roberts has proposed merging lp:~alisonken1/openlp/pjlink2-f into lp:openlp.
Commit message:
PJLink class 2 update F
Requested reviews:
OpenLP Core (openlp-core)
For more details, see:
https://code.launchpad.net/~alisonken1/openlp/pjlink2-f/+merge/326265
-- Added '# noqa' for flake8 to i
26 matches
Mail list logo
