- Remove dead upload code missed in previous pass
- api: Remove deprecated /history/ endpoints
- Move revision log limit in conf, fix behaviour
- converters: ignore ctime in content conversions
Details
- Reviewers
zack - Group Reviewers
Reviewers - Maniphest Tasks
- T431: Change API documentation method to drop Flask-API
- Commits
- R65:5a873b3c3ba6: Move revision log limit in conf, fix behaviour
R65:6007da59720f: Remove dead file upload code missed in D76
R65:d69319d8679e: converters: ignore ctime in content conversions
R65:60abefb44c1d: api: Remove deprecated /history/ endpoints
rDWAPPS5a873b3c3ba6: Move revision log limit in conf, fix behaviour
rDWAPPSd69319d8679e: converters: ignore ctime in content conversions
rDWAPPS60abefb44c1d: api: Remove deprecated /history/ endpoints
rDWAPPS6007da59720f: Remove dead file upload code missed in D76
Diff Detail
- Repository
- rDWAPPS Web applications
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Event Timeline
Great clean up commit!
I've just requested some minor changes here and there
swh/web/ui/backend.py | ||
---|---|---|
221 ↗ | (On Diff #317) | it'd be more pythonic to test this as "if not rev_list" |
swh/web/ui/converters.py | ||
196–201 ↗ | (On Diff #317) | The del() here is bad: content is aliased here, so you will end up modifying in place the argument passed to from_content(), which could be very surprising for the caller. Please copy the dictionary instead (or slice the needed parts of it before returning it further, or equivalent). |
swh/web/ui/service.py | ||
297 | This magic 25 value appears in too many places, I've counted at least 3.
| |
swh/web/ui/tests/test_app.py | ||
50–51 | Default configuration values will be added to this test configuration, right? If so, you can probably avoid setting max_log_revs explicitly here. |
swh/web/ui/tests/test_app.py | ||
---|---|---|
50–51 | I believed so, but found out it wasn't the case when running the tests. I'm not quite sure why though, since the next lines should use the conf of the actual app if I have it running at the same time, which I am. |