ail-project / ail-feeder-activity-pub

External ActivityPub feeder for AIL-framework.
GNU Affero General Public License v3.0
5 stars 0 forks source link

Quick code review #8

Open Rafiot opened 3 years ago

Rafiot commented 3 years ago
FBroy commented 3 years ago

Thank you very much for the review! I've adapted the ticked proposals in the last commit.

For the remaining 2:

Rafiot commented 3 years ago

Fair, classes/functions would only make sense if you want to make it a library, and I'm not sure it's the plan. I guess it will depends what you're going to do with the project on the long run. I like the classes/methods approach because it makes it easier to comment describe what's going on, and keeps the scripts more readable. But again, if you're not planning to have the code used as a library, it makes little sense.

Yeah, I mean these kind of calls, or that one: https://github.com/ail-project/ail-feeder-activity-pub/blob/master/bin/feeder.py#L278 The empty dictionary case should not happen, but you always, sometimes, ends up in a situation where does, the script fails and it's a bit annoying. So it's better to just have a check before.

FBroy commented 3 years ago

I'll add a check for those and notify you. Thanks again!