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

[Improvement] Add a job or step for shell script style in Github Action #2062

Closed
2 of 3 tasks
yaooqinn opened this issue Mar 8, 2022 · 12 comments
Closed
2 of 3 tasks
Assignees
Labels
good first issue beginner skills required help wanted kind:infra license, community building, project builds, asf infra related, etc. module:misc
Milestone

Comments

@yaooqinn
Copy link
Member

yaooqinn commented Mar 8, 2022

Code of Conduct

Search before asking

  • I have searched in the issues and found no similar issues.

What would you like to be improved?

shell script style consistency

How should we improve?

Maybe adding https://github.com/marketplace/actions/shell-linter in sytle.yaml is a good choice, or something else

Are you willing to submit PR?

  • Yes I am willing to submit a PR!
@yaooqinn
Copy link
Member Author

yaooqinn commented Mar 8, 2022

cc @RishiKumarRay, can you help with this?

@yaooqinn yaooqinn added good first issue beginner skills required help wanted kind:infra license, community building, project builds, asf infra related, etc. module:misc labels Mar 8, 2022
@yaooqinn yaooqinn added this to the v1.6.0 milestone Mar 8, 2022
@RishiKumarRay
Copy link
Contributor

@yaooqinn yeah i can try , can you elaborate what needs to be done ? Please, i am not able to understand completely

@yaooqinn
Copy link
Member Author

yaooqinn commented Mar 8, 2022

It is a style checker for code written in shell

For example, Kyuubi is mainly written in scala and java, so we already have style checkers for them, see https://github.com/apache/incubator-kyuubi/blob/master/.github/workflows/style.yml#L49

We need to introduce a consistent style/format for the existing and coming shell scripts, like bin/kyuubi.

@RishiKumarRay
Copy link
Contributor

@yaooqinn ok, let me try this , assign me also the path of the scripts that needs to be checked is bin/kyuubi ? Or path where the scripts are present is different.

@yaooqinn
Copy link
Member Author

yaooqinn commented Mar 8, 2022

You may check the repo yourself, this way is better for getting familiar with the codebase

@yaooqinn yaooqinn modified the milestones: v1.6.0, v1.7.0 Nov 22, 2022
@davidyuan1223
Copy link
Contributor

hello,i want slove the problem.can i try?

@pan3793
Copy link
Member

pan3793 commented Dec 30, 2022

@xiaoyuandajian of course, feel free to send PR~

@davidyuan1223
Copy link
Contributor

@pan3793 thanks

@davidyuan1223
Copy link
Contributor

@pan3793 hello, this is my first pr, I try it in my fork project, I found there are something wrong, it's my problems or the shell script?
image

@davidyuan1223
Copy link
Contributor

this is update script, I use shell-linter
image

@pan3793
Copy link
Member

pan3793 commented Dec 30, 2022

The shell-linter's doc say:

severity

Optional. Specify minimum severity of errors to consider [style, info, warning, error]. Default: style

So it's expected to report some style issue since we never use any shell linter to check that.

@davidyuan1223
Copy link
Contributor

ok, thanks

davidyuan1223 added a commit to davidyuan1223/kyuubi that referenced this issue Jan 4, 2023
davidyuan1223 added a commit to davidyuan1223/kyuubi that referenced this issue Jan 4, 2023
davidyuan1223 added a commit to davidyuan1223/kyuubi that referenced this issue Jan 4, 2023
davidyuan1223 added a commit to davidyuan1223/kyuubi that referenced this issue Jan 4, 2023
davidyuan1223 added a commit to davidyuan1223/kyuubi that referenced this issue Jan 4, 2023
@pan3793 pan3793 closed this as completed in 2dac364 Jan 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue beginner skills required help wanted kind:infra license, community building, project builds, asf infra related, etc. module:misc
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants