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

Workflow to format and lint code #235

Closed
wants to merge 1 commit into from
Closed

Workflow to format and lint code #235

wants to merge 1 commit into from

Conversation

Tetrax-10
Copy link
Collaborator

@Tetrax-10 Tetrax-10 commented Jan 25, 2024

No code has been changed, its all just formatted except 3 css files.

Please don't merge any pr before this pr. It will cause huge conflicts.

These files had errors for ages and no one noticed it. Fortunately now we have eslint and prettier to detect it.

  1. Theme-ModernDark.css - line 100
  2. Theme-Night.css -line 92
  3. Theme-PulsarLight.css - line 589

The workflow has been fully tested on this fork: Tetrax-10/CommunityScripts-Test

PR test - https://github.com/Tetrax-10/CommunityScripts-Test/pull/1

@Tetrax-10
Copy link
Collaborator Author

The failed workflow is from the stashapp/CommunityScripts main branch and not from this pr branch

Copy link
Collaborator

@WithoutPants WithoutPants left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I appreciate the effort made here, but I disagree with some of the decisions that have been made.

I don't like the idea of formatting and committing code automatically. This has generally been the responsibility of the author, and in the other repos the actions only validate that the format is correct. Refer to how it's done in the UI in the stash repo.

The prettier options used here are inconsistent with those in the other repositories. We use a tab width of 2 elsewhere. Prettier has been applied to the github yml files, and I don't think it's an improvement.

I'm not sure that applying prettier to the .md files is a good idea.

@Tetrax-10
Copy link
Collaborator Author

I will re-implement this feature by adhering to those conditions and submit a new pull request.

@Tetrax-10 Tetrax-10 closed this Jan 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants