build: set up GitHub app PRLint

[mistercrunch]

Sets up gitlint, which enforces Conventional Commits and sets up the proper post commit hook to enforce it.

Also modifying the PR template to remove redundant annoying checkbox section. Once we adopt, I'm hoping we can modify our bot to auto-label based on the commit type.

This requires a bit more discipline upfront, but should lead to a cleaner git log in master, meaning better release notes and such.

• waiting for Apache infra to install PRLint Github App. JIRA

Sets up PRLint, with a single rule matching Conventional
Commits

Also modifying the PR template to remove redundant annoying checkbox section.

Once we adopt, I'm hoping we can modify our bot to auto-label based on the commit type.

[codecov-io]

Codecov Report

Merging #9655 into master will increase coverage by 6.74%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #9655      +/-   ##
==========================================
+ Coverage   64.03%   70.77%   +6.74%     
==========================================
  Files         532      587      +55     
  Lines       28897    30460    +1563     
  Branches     2758     3121     +363     
==========================================
+ Hits        18503    21558    +3055     
+ Misses      10211     8788    -1423     
+ Partials      183      114      -69     

Flag	Coverage Δ
#cypress	53.66% <ø> (+<0.01%)	⬆️
#javascript	59.00% <ø> (?)
#python	70.93% <ø> (+0.37%)	⬆️

Impacted Files	Coverage Δ
superset-frontend/src/setup/setupPlugins.ts	8.00% <0.00%> (-92.00%)	⬇️
superset/connectors/connector_registry.py	86.66% <0.00%> (-4.45%)	⬇️
superset/utils/pandas_postprocessing.py	88.32% <0.00%> (-1.98%)	⬇️
superset-frontend/src/setup/setupApp.ts	25.00% <0.00%> (-1.93%)	⬇️
superset/models/slice.py	83.78% <0.00%> (-1.78%)	⬇️
superset/db_engine_specs/mssql.py	93.02% <0.00%> (-1.10%)	⬇️
superset/models/core.py	86.13% <0.00%> (-0.73%)	⬇️
superset/views/core.py	75.13% <0.00%> (-0.12%)	⬇️
superset/security/manager.py	89.12% <0.00%> (-0.03%)	⬇️
superset/viz.py	71.49% <0.00%> (ø)
... and 228 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5e4c291...12035bd. Read the comment docs.

[ktmud]

FYI, superset-ui actually disabled commit lint recently, because we felt it's not really necessary given that we already had Semantic Pull Request check and will almost always do squash merge anyway.

Considering incubator-superset also only do squash merge, if this is to be added, would it be possible to enforce the checks only on the master branch?

[craig-rueda]

Agree. I'm all for making the commit log more readable. Typically, when squashing, the commits that are actually being squashed are appended into the squash commit's message. Let's give it a shot...

[nytai]

I agree with @ktmud a per check would be preferable. We can even enforce semantic commits on the actual PR. A local pre-commit check seems like it could get annoying really quickly. I usually fixup my commits or reword them before opening a pr or when submitting pr for final review, especially given the commit messages get squashed into the commit that's finally merged. However, I commit often when developing locally for things like moving branches or commits I intend to fixup. Having to write a message that'll pass commit lint, not to mention running commit lint, when I just want to save my work and move to another branch seems tedious.

[mistercrunch]

I agree local post-commit hook is heavy, there's a way to add a regex to skip something like wip.*.

It's true that ultimately with that "squash & merge" github policy it's the PR title that matters most. Maybe we force a PR title check to merge instead?

[mistercrunch]

Oh! how about this: https://github.com/apps/prlint !?

[ktmud]

Oh! how about this: github.com/apps/prlint !?

Looks legit! Seems more powerful than Semantic Pull Requests that superset-ui is using.

[dpgaspar]

PRLint Looks good. Something like this would really help on patch releases and making the changelog more readable and parseable

[villebro]

I like it!

[mistercrunch]

Here: https://issues.apache.org/jira/browse/INFRA-20236

[mistercrunch]

Leaving open until https://issues.apache.org/jira/browse/INFRA-20236 closes

[mistercrunch]

Alright I got Apache infra to install PRLint, with a single rule matching Conventional
Commits and configured it in this PR!

We can add more rules around labeling, size, assignment, body, ... Worth checking out their docs to see what's possible.

[dpgaspar]

awesome! let's merge it?

[dpgaspar]

LGTM, we can iterate over this on subsequent PR's to add feature around labels etc...

[john-bodley]

@mistercrunch why the removal of the reviewers section?

[mistercrunch]

oh, you can just do this on the PR itself and get proper notifications related to that:

The person opening the PR probably doesn't know who should review, so having this in the template makes outsiders feel even more that way.

There are ways to auto-assign based on which folders/modules are touched we could use in the future.

[ktmud]

It seems only repo collaborators can request reviewers from there. Do all committers get notified when a new PR is created?

[mistercrunch]

Oh, we should add a feature to let anyone assign PRs through comments on the apache superset Github bot:
https://github.com/apache-superset/superset-github-bot

It can be a way to work around missing perms for contributors (non-committers).

The labeling feature broke recently for reasons I could not understand (felt like unicode-related around parsing the emoji). Spent 2 hours and gave up.