fixed linter warnings & moved linting to a separate CI job

[LeXofLeviafan]

As per discussion in f00348e & #676

[jarun]

Why can't we remove the else? What is this syntax try ... else? Why can't it be avoided?

[LeXofLeviafan]

Why can't we remove the else?

I don't follow… Removing it is exactly what I did :sweat_smile:

What is this syntax try ... else?

else after anything other than if generally means "if the block executed normally"; i.e. else after a loop means "if loop exited on condition (not with a break)", and else after try means "if no error was raised".

…Yes, this isn't obvious at a glance, so this syntax is somewhat discouraged; it can be replicated explicitly by using boolean flags or return (which is what's going on here, so linter detected that using else here isn't necessary).

[jarun]

Wouldn't the following work?

        except Exception as ex: 
            if not self.handle_view_exception(ex):
                msg = "Failed to create record."
                flash(
                    gettext("%(msg)s %(error)s", msg=msg, error=str(ex)),
                    "error",
                )
                LOG.exception(msg)
            return False

        self.after_model_change(form, model, True)
        return model

[LeXofLeviafan]

…You mean exactly like in this pull-request?

[jarun]

OK. I saw the initial moved linting to a separate CI job changes. Why do we need that?

[LeXofLeviafan]

Maybe [pylint] should be moved into a separate check (along with flake8); I doubt that it'd be much different between Python versions, and the result would be making clear whether it's linting or tests that doesn't pass.

That is, as of now it's hard to tell (without digging into logs) if the checks fail because of linting, or because the code doesn't work correctly.

[jarun]

We do need to check the logs on a failure. Please make the code change only.

[LeXofLeviafan]

At this moment, you couldn't even tell that master is passing all tests because it doesn't pass linting; doing the linting separately would make it much easier to tell what needs to be fixed (and reduce the amount of attempts necessary to fix erroneous pull-requests).

[jarun]

OK. If that makes things simpler.