flipperdevices / flipper-application-catalog

Flipper Application Catalog
615 stars 159 forks source link

Integrate `ufbt format` into build pipeline #65

Closed akopachov closed 1 year ago

akopachov commented 1 year ago

I saw it at least couple of times people were submitting apps which's source code is not formatted using ufbt format command. Here are two questions I have:

  1. Why do this catalog care about formatting at all given that the apps are maintained by their maintainers and why don't you let them keep those formatting which they prefer?
  2. If p.1 by some reason is super-duper important for this catalog, why don't you just integrate ufbt format into build pipeline you have to keep yourself happy and still let maintainers have formatting they prefer.
leedave commented 1 year ago

I frankly wouldn't mind having a setup in VSC that automatically does this. I know it should be possible using the .clang-format file from the Flipper repo, but after hours of trying, digging through VSC errors, not finding any helpful documentation, someone told me I can save the effort and just do it with uFbt.

akopachov commented 1 year ago

There are multiple workarounds (one of the examples - what I did for Authenticator app)

However these are workarounds and they are not answering two questions above:

  1. Why this catalog care about what formatting maintainers of 3rd party apps are using?
  2. If they care that much - why don't this catalog automatically format sources according to its needs as a part of building pipeline?
hedger commented 1 year ago

@akopachov @leedave Formatting code for your app is as easy as running a single ufbt format command in its folder.

We do a quick code review of apps that are submitted to the catalog - and having a well-formatted code greatly helps.

However, we've temporarily disabled the enforcement of linter checks for apps in the catalog, based on the feedback. It may return, if new apps and updates will be hard to review without being formatted.

akopachov commented 1 year ago

I see your points.

However I still do not quite understand why don't you guys run ufbt format instead of ufbt lint as a part of build script you are using to prepare artifacts for this catalog? Isn't this going to keep happy everybody?

leedave commented 1 year ago

Actually having a struggle with ufbt. I normally use fbt as it works easy under windows. Somehow ufbt does not, so I need to run it from WSL2 which is quite a detour.