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

Add a CI Job to check proper formatting while submitting a PR #43335

Closed
Gauravpadam opened this issue Oct 5, 2023 · 24 comments
Closed

Add a CI Job to check proper formatting while submitting a PR #43335

Gauravpadam opened this issue Oct 5, 2023 · 24 comments
Assignees
Labels
kind/feature Categorizes issue or PR as related to a new feature. language/en Issues or PRs related to English language lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. sig/docs Categorizes an issue or PR as relevant to SIG Docs.

Comments

@Gauravpadam
Copy link
Member

This is a Feature Request

What would you like to be added
We should add a CI Job to the repository to ensure that our contributors are following the formatting rules while submitting their PRs

Why is this needed
The merge of #43279 defines manual wrapping for ease of review and localization. The CI feature will make sure the style guide is adapted in our future PRs

@Gauravpadam Gauravpadam added the kind/feature Categorizes issue or PR as related to a new feature. label Oct 5, 2023
@k8s-ci-robot
Copy link
Contributor

This issue is currently awaiting triage.

SIG Docs takes a lead on issue triage for this website, but any Kubernetes member can accept issues by applying the triage/accepted label.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Oct 5, 2023
@Gauravpadam
Copy link
Member Author

/assign
I'll be working on this

@thisisharrsh
Copy link
Contributor

/language en
/sig docs

@k8s-ci-robot k8s-ci-robot added language/en Issues or PRs related to English language sig/docs Categorizes an issue or PR as relevant to SIG Docs. labels Oct 5, 2023
@sftim
Copy link
Contributor

sftim commented Oct 5, 2023

Make sure to use Prow for the checks (not GitHub Actions, which as a project we avoid).

@Gauravpadam
Copy link
Member Author

Thanks for the heads up, I'll keep that in mind

Make sure to use Prow for the checks (not GitHub Actions, which as a project we avoid).

@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue as fresh with /remove-lifecycle stale
  • Close this issue with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 29, 2024
@Okabe-Junya
Copy link
Member

Hi folks !!

Is this issue currently in progress? I'm also interested in the implementation of CI. Here are some points I'm curious about:

  • Will you implement simple shellscripts, or use some tools like markdownlint?
  • Do you plan to introduce CI for all languages?
  • How will you address the issue of false positives in natural language CI?
  • Do you plan to include in the required status?
  • Do you plan to offer it as an experimental feature?
    • For example, how about enabling a beta version for specific languages?

Regards :)

@Okabe-Junya
Copy link
Member

I think the simple first step would be to ensure:

  • Checking no unnecessary spaces at the end of lines
  • Checking exactly only one newline at the end of files.

I think this would be effective for all languages

@Okabe-Junya
Copy link
Member

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 30, 2024
@Gauravpadam
Copy link
Member Author

Thanks for the insights @Okabe-Junya

I think this is one of those issues where sig-testing can help us. I'll rekindle this by reaching out on their slack channel

btw, rolling out a beta for specific languages sgtm. We can start by clearing linting checks

@Affan-7
Copy link
Contributor

Affan-7 commented Jan 30, 2024

Are we going to use a linting tool for text? Or create one?

@Gauravpadam
Copy link
Member Author

What would you suggest @Affan-7?

@stmcginnis
Copy link
Contributor

I have had good luck with markdownlint-cli in past projects. The tricky part will be configuring it so it helps more than hinders contributions. The default ruleset is fairly struct on markdown adherence, even the rendered output is fine. Adding that to an existing set of files would likely highlight a LOT of issues. Some will be legitimate things to fix, while others would be trivial cleanup that would be less important.

The cluster-api-provider-vmware maintains a containerized image of this that makes it very easy to perform checks:

https://github.com/vmware-tanzu/community-edition/blob/main/hack/check/check-mdlint.sh#L16

@Okabe-Junya
Copy link
Member

The tricky part will be configuring it so it helps more than hinders contributions

I think too. I also think we should start with the minimal configuration (meaning, we should initially have a rule set that produces almost no false positives).

How about starting with spell check and link verification, for example?

@Okabe-Junya
Copy link
Member

/scripts contains several excellent scripts. Running these in CI would be an easier step than introducing new tools.

(If we were to introduce a new linter, we would have to spend a lot of time discussing its rule set.)

@Affan-7
Copy link
Contributor

Affan-7 commented Jan 31, 2024

Kubernetes have a lot of formatting standards, you can refer style guide. Can we configure markdownlint-cli to implement things like linting for headings and titles and other similar things?

@Gauravpadam
Copy link
Member Author

How about starting with spell check and link verification, for example?

This sgtm. k8s/test-infra has all the ci jobs atm. If this works out, We can turn this into an umbrella issue and integrate different style guide checks.

@Okabe-Junya
Copy link
Member

Sounds good!!

@Gauravpadam, What about i18n? Should we initially target only English, and if it works well, then introduce it in other languages based on the decision of each localization team?

Meaning, I think what we need to do is to add a simple CI workflow file for English documentation to k/test-infra?

@Gauravpadam
Copy link
Member Author

@Okabe-Junya Like you've mentioned earlier, some checks are common for all types of files, writing a workflow for them should be a quick win

For i18n specifically? alphabetical ordering checks is what we should aim for initially imo

@Okabe-Junya
Copy link
Member

Looks good!!

Could you let me know in this issue when you open a PR to k/test-infra?

@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue as fresh with /remove-lifecycle stale
  • Close this issue with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 2, 2024
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue as fresh with /remove-lifecycle rotten
  • Close this issue with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jun 1, 2024
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Reopen this issue with /reopen
  • Mark this issue as fresh with /remove-lifecycle rotten
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close not-planned

@k8s-ci-robot
Copy link
Contributor

@k8s-triage-robot: Closing this issue, marking it as "Not Planned".

In response to this:

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Reopen this issue with /reopen
  • Mark this issue as fresh with /remove-lifecycle rotten
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close not-planned

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@k8s-ci-robot k8s-ci-robot closed this as not planned Won't fix, can't repro, duplicate, stale Jul 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. language/en Issues or PRs related to English language lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. sig/docs Categorizes an issue or PR as relevant to SIG Docs.
Projects
None yet
Development

No branches or pull requests

8 participants