-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Short report: Mark postponed risks #693
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I approve, but I suspect some improvements seem desirable.
If you have no time to do that now, please remember that for the future.
src/euphorie/client/docx/compiler.py
Outdated
@@ -815,6 +817,9 @@ def set_modules_rows(self, data): | |||
count = 0 | |||
for risk in risks: | |||
answer = risk.get("justifiable", "") | |||
postponed = risk.get("postponed", False) | |||
if not answer and postponed: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no point in getting postponed if answer is falsish, but ok...
src/euphorie/client/docx/compiler.py
Outdated
) | ||
cell = row_risk.cells[self.risk_answer_col] | ||
cell.text = self.justifiable_map.get(answer) or "" | ||
font = self.justifiable_font.get(answer) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this increases the complexity of the method, which is already complex. But ok...
Probably adding a separate method, expanded in the DocxCompilerShort
would have been better.
But I am not able to really judge by just reading the PR.
I refactored a little. Maybe the whole module would benefit from some more refactoring... I'll see if I can sneak something in.
The whole point of code reviews is that a single person cannot think of all possible improvements all the time. Asking me to do this seems both condescending and pointless to me. |
Sorry for that, I thought I was doing good |
syslabcom/scrum#1852