3:24 a.m.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-tflink.rhcloud.com/r/2/#review6
-----------------------------------------------------------
blockerbugs/config.py
<http://reviewboard-tflink.rhcloud.com/r/2/#comment5>
Why is this commented out?
blockerbugs/controllers/main.py
<http://reviewboard-tflink.rhcloud.com/r/2/#comment6>
And why is this commented out? :)
blockerbugs/util/bz_interface.py
<http://reviewboard-tflink.rhcloud.com/r/2/#comment7>
The url could be loaded from the config, right?
blockerbugs/util/bz_interface.py
<http://reviewboard-tflink.rhcloud.com/r/2/#comment8>
Just curious, why is this not loaded in the constructor?
- Martin Krizek
On Feb. 7, 2013, 9:01 p.m., Tim Flink wrote:
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-tflink.rhcloud.com/r/2/
-----------------------------------------------------------
Review request for blockerbugs.
Bugs: 334
https://fedorahosted.org/fedora-qa/ticket/334
Repository: blockerbugs
Description
-------
Initial code for blocker submission page
Diffs
-----
testing/test_bugchange.py PRE-CREATION
init_f18db.sh c20df66c4b4a8f38ab79a1b61043d26de112f71c
init_db.sh PRE-CREATION
blockerbugs/util/bz_interface.py 2bae79db2d3a1206b7de1cee5f005f137ba2d6f3
blockerbugs/templates/thanks.html PRE-CREATION
blockerbugs/templates/propose_bug.html PRE-CREATION
blockerbugs/templates/base_nav.html f3a89798a9dadf297e722c067ad3795a72ebbf74
blockerbugs/controllers/main.py e7525f8433d74802f7c0cb2d719f0abdc13431c4
blockerbugs/controllers/forms.py 669d78a5030904d4559d6d68af093bd6f096888b
blockerbugs/config.py 009e7a97f57e31fcc499dba87fe7d33e41252a2c
blockerbugs/__init__.py ad051ca2523aa70ca2d4d26843e9a2bdf0b5cd00
Diff: http://reviewboard-tflink.rhcloud.com/r/2/diff/
Testing
-------
Thanks,
Tim Flink
8:52 a.m.
New subject: Review Request 2: Initial Blocker Review Code
On Feb. 22, 2013, 9:24 a.m., Martin Krizek wrote:
> blockerbugs/util/bz_interface.py, line 185
> <http://reviewboard-tflink.rhcloud.com/r/2/diff/1/?file=19#file19line185>
>
> Just curious, why is this not loaded in the constructor?
Part of it is a stylistic thing - I don't like constructors that load remote data.
I've run into issues in the past with constructors that take too long or load a bunch
of data wrt testability. It's not so much of an issue here since the loading can be
overridden with a mock bz object but like I said, it's partially a stylistic thing on
my part.
Do you think that the code would be easier to read or better if that was loaded in the
constructor?
On Feb. 22, 2013, 9:24 a.m., Martin Krizek wrote:
> blockerbugs/util/bz_interface.py, line 48
> <http://reviewboard-tflink.rhcloud.com/r/2/diff/1/?file=19#file19line48>
>
> The url could be loaded from the config, right?
Yeah and it probably should. When the code was first written, the bugzilla URL wasn't
in the app config and while normally, I'd say "let's not load the whole app
just to get a config value", we're already doing that implicitly when we load the
DB models so it's not a big issue.
- Tim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-tflink.rhcloud.com/r/2/#review6
-----------------------------------------------------------
On Feb. 22, 2013, 2:41 p.m., Tim Flink wrote:
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-tflink.rhcloud.com/r/2/
-----------------------------------------------------------
(Updated Feb. 22, 2013, 2:41 p.m.)
Review request for blockerbugs.
Bugs: 334
https://fedorahosted.org/fedora-qa/ticket/334
Repository: blockerbugs
Description
-------
Initial code for blocker submission page
Diffs
-----
testing/test_bugchange.py PRE-CREATION
init_f18db.sh c20df66c4b4a8f38ab79a1b61043d26de112f71c
init_db.sh PRE-CREATION
blockerbugs/util/bz_interface.py 2bae79db2d3a1206b7de1cee5f005f137ba2d6f3
blockerbugs/templates/thanks.html PRE-CREATION
blockerbugs/templates/propose_bug.html PRE-CREATION
blockerbugs/templates/base_nav.html f3a89798a9dadf297e722c067ad3795a72ebbf74
blockerbugs/controllers/main.py e7525f8433d74802f7c0cb2d719f0abdc13431c4
blockerbugs/controllers/forms.py 669d78a5030904d4559d6d68af093bd6f096888b
blockerbugs/config.py 009e7a97f57e31fcc499dba87fe7d33e41252a2c
blockerbugs/__init__.py ad051ca2523aa70ca2d4d26843e9a2bdf0b5cd00
Diff: http://reviewboard-tflink.rhcloud.com/r/2/diff/
Testing
-------
Thanks,
Tim Flink
5:03 a.m.
New subject: Review Request 2: Initial Blocker Review Code
On Feb. 22, 2013, 9:24 a.m., Martin Krizek wrote:
> blockerbugs/util/bz_interface.py, line 185
> <http://reviewboard-tflink.rhcloud.com/r/2/diff/1/?file=19#file19line185>
>
> Just curious, why is this not loaded in the constructor?
Tim Flink wrote:
Part of it is a stylistic thing - I don't like constructors that load remote
data. I've run into issues in the past with constructors that take too long or load a
bunch of data wrt testability. It's not so much of an issue here since the loading can
be overridden with a mock bz object but like I said, it's partially a stylistic thing
on my part.
Do you think that the code would be easier to read or better if that was loaded in
the constructor?
Not necessarily. I was just curious if there is another reason other than loading remote
data.
- Martin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-tflink.rhcloud.com/r/2/#review6
-----------------------------------------------------------
On Feb. 22, 2013, 2:41 p.m., Tim Flink wrote:
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-tflink.rhcloud.com/r/2/
-----------------------------------------------------------
(Updated Feb. 22, 2013, 2:41 p.m.)
Review request for blockerbugs.
Bugs: 334
https://fedorahosted.org/fedora-qa/ticket/334
Repository: blockerbugs
Description
-------
Initial code for blocker submission page
Diffs
-----
testing/test_bugchange.py PRE-CREATION
init_f18db.sh c20df66c4b4a8f38ab79a1b61043d26de112f71c
init_db.sh PRE-CREATION
blockerbugs/util/bz_interface.py 2bae79db2d3a1206b7de1cee5f005f137ba2d6f3
blockerbugs/templates/thanks.html PRE-CREATION
blockerbugs/templates/propose_bug.html PRE-CREATION
blockerbugs/templates/base_nav.html f3a89798a9dadf297e722c067ad3795a72ebbf74
blockerbugs/controllers/main.py e7525f8433d74802f7c0cb2d719f0abdc13431c4
blockerbugs/controllers/forms.py 669d78a5030904d4559d6d68af093bd6f096888b
blockerbugs/config.py 009e7a97f57e31fcc499dba87fe7d33e41252a2c
blockerbugs/__init__.py ad051ca2523aa70ca2d4d26843e9a2bdf0b5cd00
Diff: http://reviewboard-tflink.rhcloud.com/r/2/diff/
Testing
-------
Thanks,
Tim Flink
4092
days inactive
4095
days old
autoqa-devel@lists.fedorahosted.org
2 comments
1 participants
participants (1)
-
Martin Krizek