Setup Gitlint. #645

LoopThrough-i-j · 2021-01-15T20:47:54Z

No description provided.

neiljp

@LoopThrough-i-j This looks broadly good to me, though other than the doc updates I'd like to see noted how much is different to in zulip/zulip, and how much is a straight copy (as per my comments).

ZT settled on a slightly different commit format, but I know the API repo matches the main one. That said, I personally prefer the more explicit nature of ZT git-linting since it gives more specific feedback on what is missing/wrong. That may be a discussion for somewhere else, however.

neiljp · 2021-01-15T21:18:43Z

.gitlint

@@ -0,0 +1,13 @@
+[general]
+ignore=title-trailing-punctuation, body-min-length, body-is-missing, title-imperative-mood


Did you mean to exclude title-imperative-mood? Is this is from zulip/zulip? It seems strange to exclude a rule that's added in the extra rules.

I am taking a look and trying to make the required changes. Thanks for the review.

This is wrong in zulip/zulip; see zulip/zulip#17393. Once that merges, please bring those changes into this PR.

tools/lint-commits

LoopThrough-i-j · 2021-02-25T01:34:09Z

@alexmv The changes are made. I think they look good to be merged now, still consider reviewing and add any more suggestions if required.

README.md

timabbott · 2021-02-25T02:25:56Z

README.md

@@ -25,14 +25,18 @@ and [commit guidelines](https://zulip.readthedocs.io/en/latest/contributing/vers
 1. Fork and clone the Git repo:
   `git clone https://github.com/<your_username>/python-zulip-api.git`

-2. Make sure you have [pip](https://pip.pypa.io/en/stable/installing/)
+2. Setup upstream and fetch upstream/master (Required for Gitlint):


Let's spell it [Gitlint](https://jorisroovers.com/gitlint/)

README.md

timabbott · 2021-02-25T02:27:53Z

For the files copied from zulip/zulip, can you add a comment noting that's the case at the top of the file? And also mention if anything has changed. Something like:

# This file is copied from the original at tools/foobar in zulip/zulip.
# Please don't edit here; instead update the zulip/zulip copy and then resync this file.

README.md

tools/lint-commits

timabbott · 2021-03-04T19:36:06Z

tools/lint-commits

+if [[ "
+$(git remote -v)
+" =~ '
+'([^[:space:]]*)[[:space:]]*(https://github\.com/|ssh://git@github\.com/|git@github\.com:)zulip/python-zulip-api(\.git|/)?\ \(fetch\)'


Probably we should do a commit to zulip/zulip that makes the repository name a variable at the top of the file.

Okay, we can do that.

Set and Fetch upstream is required for gitlint, while setting up development env.

Setup gitlint for developers to write well formatted commit messages. Note: .gitlint, gitlint-rules.py and lint-commits are taken directly from zulip/zulip with minor changes.

timabbott · 2021-03-04T19:39:28Z

Resolved the minor merge conflict when rebasing and merged, thanks @LoopThrough-i-j!

One thing I noticed is that mypy prints a bunch of debug output that it doesn't on zulip/zulip; can you look into whatever flag we need to suppress that?

mypy      | Running mypy for `zulip`.
mypy      | Success: no issues found in 69 source files
mypy      | Running mypy for `zulip_bots`.
mypy      | Success: no issues found in 75 source files
mypy      | Running mypy for `zulip_botserver`.
mypy      | Success: no issues found in 7 source files
mypy      | Running mypy for `tools`.
mypy      | Success: no issues found in 2 source files

LoopThrough-i-j · 2021-03-04T19:47:09Z

Sure will check and push a fix. Thanks for the merge.

andersk · 2021-03-04T22:08:10Z

tools/lint

@@ -15,6 +15,7 @@ EXCLUDED_FILES = [
 def run() -> None:
    parser = argparse.ArgumentParser()
    add_default_linter_arguments(parser)
+    parser.add_argument('--no-gitlint', action='store_true', help='Disable gitlint')


zulint already has an option for this: #659.

Ok, I will create a PR soon, fixing the required changes.

zulipbot added the size: XL label Jan 15, 2021

neiljp reviewed Jan 15, 2021

View reviewed changes

LoopThrough-i-j force-pushed the git-lint branch 2 times, most recently from 537fcf5 to 5177377 Compare January 15, 2021 22:00

LoopThrough-i-j closed this Feb 23, 2021

LoopThrough-i-j reopened this Feb 23, 2021

LoopThrough-i-j force-pushed the git-lint branch 3 times, most recently from 1889390 to 4b4f6b4 Compare February 25, 2021 01:32

LoopThrough-i-j requested a review from alexmv February 25, 2021 01:34

timabbott reviewed Feb 25, 2021

View reviewed changes

README.md Outdated Show resolved Hide resolved

timabbott reviewed Feb 25, 2021

View reviewed changes

README.md Outdated Show resolved Hide resolved

LoopThrough-i-j force-pushed the git-lint branch 3 times, most recently from aa02421 to 89b7eae Compare February 25, 2021 03:29

LoopThrough-i-j requested a review from timabbott February 25, 2021 03:35

alexmv reviewed Feb 25, 2021

View reviewed changes

README.md Outdated Show resolved Hide resolved

alexmv reviewed Feb 25, 2021

View reviewed changes

tools/lint-commits Outdated Show resolved Hide resolved

LoopThrough-i-j changed the title ~~Lint: Setup gitlint.~~ [WIP]Lint: Setup gitlint. Feb 25, 2021

LoopThrough-i-j force-pushed the git-lint branch 2 times, most recently from 7ceb832 to 7585161 Compare February 25, 2021 04:22

LoopThrough-i-j requested a review from alexmv February 25, 2021 04:23

LoopThrough-i-j changed the title ~~[WIP]Lint: Setup gitlint.~~ Setup Gitlint. Feb 25, 2021

timabbott reviewed Mar 4, 2021

View reviewed changes

LoopThrough-i-j added 3 commits March 4, 2021 11:36

docs: Add set and fetch upstream.

f8cd424

Set and Fetch upstream is required for gitlint, while setting up development env.

lint: Setup gitlint.

cce18ed

Setup gitlint for developers to write well formatted commit messages. Note: .gitlint, gitlint-rules.py and lint-commits are taken directly from zulip/zulip with minor changes.

lint: Remove gitlint from static analysis.

c4a78d0

timabbott force-pushed the git-lint branch from 7585161 to c4a78d0 Compare March 4, 2021 19:38

timabbott merged commit c4a78d0 into zulip:master Mar 4, 2021

andersk reviewed Mar 4, 2021

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Setup Gitlint. #645

Setup Gitlint. #645

LoopThrough-i-j commented Jan 15, 2021

neiljp left a comment

neiljp Jan 15, 2021

LoopThrough-i-j Jan 15, 2021

alexmv Feb 23, 2021

LoopThrough-i-j commented Feb 25, 2021

timabbott Feb 25, 2021

timabbott commented Feb 25, 2021 •

edited

Loading

timabbott Mar 4, 2021

LoopThrough-i-j Mar 4, 2021

timabbott commented Mar 4, 2021

LoopThrough-i-j commented Mar 4, 2021

andersk Mar 4, 2021

LoopThrough-i-j Mar 5, 2021

		@@ -0,0 +1,13 @@
		[general]
		ignore=title-trailing-punctuation, body-min-length, body-is-missing, title-imperative-mood

Setup Gitlint. #645

Setup Gitlint. #645

Conversation

LoopThrough-i-j commented Jan 15, 2021

neiljp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LoopThrough-i-j commented Feb 25, 2021

Choose a reason for hiding this comment

timabbott commented Feb 25, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

timabbott commented Mar 4, 2021

LoopThrough-i-j commented Mar 4, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

timabbott commented Feb 25, 2021 •

edited

Loading