Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify fmt-check #21458

Merged
merged 7 commits into from
Oct 16, 2022
Merged

Simplify fmt-check #21458

merged 7 commits into from
Oct 16, 2022

Conversation

silverwind
Copy link
Member

@silverwind silverwind commented Oct 14, 2022

fmt-check now simply does fmt before and relies on git diff like other checks like 'tidy-check' already do, so we can remove the argument in the tool that handles printing changed files.

fmt-check now simly does fmt before and relies on 'git diff' like
other checks like 'tidy-check' already do, so we can remove the
argument in the tool that handles printing changed files.
@silverwind silverwind added the skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. label Oct 14, 2022
@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Oct 14, 2022
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Oct 15, 2022
@wxiaoguang
Copy link
Contributor

The misspell package is never called

@wxiaoguang
Copy link
Contributor

Actually, after

The misspell is never used, the only sub-command call to code-batch-format is gitea-fmt, no misspell anymore.

@silverwind
Copy link
Member Author

Right, I will add misspell again.

@wxiaoguang
Copy link
Contributor

Get this merged, or wait for your time?

@silverwind
Copy link
Member Author

misspell seems be ran as part of golangci-lint now, so we can at least remove the remnants of it in code-batch-format.

misspell will not catch misspellings in JS/CSS files, I will check if something can be done to run it outside golangci-lint to fix that.

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Oct 15, 2022

Yup, I mean ... merge this PR now (it has got 2 approvals and is ready to merge), or wait for more time? 😂

@silverwind
Copy link
Member Author

Misspell now runs as part of fmt, but I think I will actually change it again because sometimes misspell's corrections may break code, so it's better to check only.

@silverwind
Copy link
Member Author

Done, now misspell also picks up JS/CSS files and runs as a check only without autocorrection as that can be dangerous.

@wxiaoguang wxiaoguang merged commit 0647df3 into go-gitea:main Oct 16, 2022
@wxiaoguang
Copy link
Contributor

--> Improve code format during build, remove code-batch-process #21471

zjjhot added a commit to zjjhot/gitea that referenced this pull request Oct 18, 2022
* upstream/main: (32 commits)
  inline gitpod image (go-gitea#21494)
  [skip ci] Updated translations via Crowdin
  Do not send notifications for draft releases (go-gitea#21451)
  Update reverse-proxies.zh-cn.md (go-gitea#21484)
  Docs: Update the feature comparison to other Git Hosting Services (go-gitea#20933)
  Add some api integration tests (go-gitea#18872)
  probe if sha before exec git (go-gitea#21467)
  Fix incorrect notification commit url (go-gitea#21479)
  Localize all timestamps (go-gitea#21440)
  [skip ci] Updated translations via Crowdin
  Add system setting table with cache and also add cache supports for user setting (go-gitea#18058)
  Return 404 when user is not found on avatar (go-gitea#21476)
  Enforce grouped NuGet search results (go-gitea#21442)
  Display total commit count in hook message (go-gitea#21400)
  Refactor GetNextResourceIndex to make it work properly with transaction (go-gitea#21469)
  Simplify fmt-check (go-gitea#21458)
  update current stable version
  1.17.3 changelog
  [skip ci] Updated translations via Crowdin
  Fix mermaid-related bugs (go-gitea#21431)
  ...
@silverwind silverwind deleted the fmtcheck branch December 17, 2022 00:26
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants