app: make the WestApp class and the run method public API

[tejlmand]

Changing the comment, as we now officially support direct use of
WestApp().

Signed-off-by: Torsten Rasmussen Torsten.Rasmussen@nordicsemi.no

---

This PR is needed by: zephyrproject-rtos/zephyr#26060
in order to correctly render the WestApp api.

[mbolivar-nordic]

Can we stick with the original plan to just document main()? Or can you provide a justification for why this is better? I don't see it but I haven't thought about it that deeply. Thanks.

[tejlmand]

Let me try to give a couple of reasons:

First the simple fact that using WestApp as a class makes it look more correctly, seen from an object oriented language point of view.
I can do:

app = WestApp()
app.run([<args>])

or simply:

WestApp().run([<args>])

so I find this more esthetic correct, than main([<args>]), cause main from where ?

Secondly, main() itself may be ambiguous.
This code:
https://github.com/zephyrproject-rtos/zephyr/blob/6af116bdb8a5e29dd3a168666c1dc11b43bcd6b3/scripts/zephyr_module.py#L186-L192
works, because the import happens inside the current method, but if you move from west.app.main import main to Line 28 instead, you will get the following error:

Traceback (most recent call last):
  File "../scripts/zephyr_module.py", line 247, in <module>
    main()
  File "../scripts/zephyr_module.py", line 193, in main
    main(['list', '--format={posixpath}'])
TypeError: main() takes 0 positional arguments but 1 was given

because main suddenly becomes ambiguous, as it is also defined here:
https://github.com/zephyrproject-rtos/zephyr/blob/6af116bdb8a5e29dd3a168666c1dc11b43bcd6b3/scripts/zephyr_module.py#L161
and used here:
https://github.com/zephyrproject-rtos/zephyr/blob/6af116bdb8a5e29dd3a168666c1dc11b43bcd6b3/scripts/zephyr_module.py#L245

so using main directly from west is ambiguous, and bound to give problems if someone imports it early.
(Technically in Python, main itself is not really ambiguous, as one main is simply overriding (and thus hiding) the other main)

Whereas,, using WestApp().run() is unambigous, and it is clearer for the reader of the code, what WestApp() is, whereas main() could be harder for a reader to follow, cause which main are we talking about.

[tejlmand]

One more thing: I see no issue in switching back to using main(), I just feel WestApp().run() is cleaner, based on reasons given above.

That was the reason for changing the code in the updated PR.

[mbolivar-nordic]

First the simple fact that using WestApp as a class makes it look more correctly, seen from an object oriented language point of view.
I can do:

app = WestApp()
app.run([<args>])

or simply:

WestApp().run([<args>])

so I find this more esthetic correct, than main([<args>]),

To me, an object oriented API implies that I can instantiate multiple WestApp objects and use them at the same time.

This is not true, because of #149.

On the other hand, nobody "in their right mind" would suspect a function named main() of doing anything else but modifying a bunch of global state.

So though it looks nicer, these looks are deceiving IMO due to this issue.

cause main from where ?

Why, from west.app.main, of course, where it was imported from.

Secondly, main() itself may be ambiguous.

If that's a problem, import ... as is a Pythonic fix:

from west.app.main import main as west_app_main

...

west_app_main()

[tejlmand]

To me, an object oriented API implies that I can instantiate multiple WestApp objects and use them at the same time.

This is not true, because of #149.

That's a valid point.

Secondly, main() itself may be ambiguous.

If that's a problem, import ... as is a Pythonic fix:

I know you can import ... as ..., and that is exactly useful when there are colliding names (or you want to import an alternative implementation of existing API, but the existing code is already referring the name of previous module implementing the API).

But main is such common name, that I found it more correct to use a West specific API, and not going through a import ... as ... cause with main it makes the code look as we are doing something we shouldn't be doing imho.
But as long as #149 is open, using main() is fine.

We could also choose to fix #149.
If #149 is fixed, which version would you prefer, main() or WestApp().run() ?

I would like to avoid switching from main --> WestApp().run() later, when #149 is fixed.
But if you prefer main() in any case, then I will switch back.

[tejlmand]

done. switched back to main()

[mbolivar-nordic]

Thanks a lot for bearing with me on this.

If you can't get the API docs working with shippable on the zephyr PR, it's fine; I will remember this when I'm writing up the 0.8.0 docs.

[tejlmand]

No problem. I prefer it takes time to settle and then do code changes, compared to having to change to often.

I think it was good discussion, although I still prefer the WestApp version, but I do acknowledge we should get #149 fixed.

[tejlmand]

btw. the docs build correctly locally:

[mbolivar-nordic]

btw. the docs build correctly locally:

Sure, we just may not be able to get this built in the zephyr CI system and nightlies until the next west release.