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

Cleanup Logic for loops #739

Closed
wants to merge 3 commits into from
Closed

Cleanup Logic for loops #739

wants to merge 3 commits into from

Conversation

admiralAwkbar
Copy link
Collaborator

So this is a crazy idea on how we get into the basic loops...
But the main idea is that we create 2 sets of data:

  • All files changed, or modified files changed
    We then use that data to see if we need to even go into the specific linter loops.
    This would help stop going into loops where the user has no code. Hopefully eliminating many random failures for languages users are not using.

Its a fun idea, and might take some more cleanup

@nvuillam
Copy link
Contributor

nvuillam commented Sep 18, 2020

I hate to be a buzz killer .... but aren't we reaching the limits of BASH scripts ?
There are so many copy/paste & redundancies, and every generic behaviour update forces to update all linter-specific code ...
Maybe you have an intern at GitHub who could rewrite all that in typescript or js ? 😈

@admiralAwkbar
Copy link
Collaborator Author

@nvuillam @GaboFDC
So this was an interesting idea:

  • Now we create the list of files for the respective arrays...
    then we use that to make sure we don't go into a linters loop unless there are files for it.
    Should help prevent issues about random linters failing... but would love your opinion

@admiralAwkbar
Copy link
Collaborator Author

@nvuillam I think I agree...
This was a super fun simple script in the beginning and only handled a few languages and was super simple.
As we have added more and more features and additional languages, that all require their small changes, this is getting to a critical point.
Unfortunately, I built this is my spare time and GitHub does not pay me to work on it, or maintain it, so we open-sourced it.

It may be an interesting idea to get some help for v4 and bring it into a more high-level language...

I had always hoped we would get some component action steps and be able to split this up to be more reasonable, but that point has not been reached yet :)

@nvuillam
Copy link
Contributor

@admiralAwkbar > I never learned to code in Python... maybe it could be an occasion to spend a week-end doing that, I'll think about it ... 😄

For my "high level" point of view, we could keep all repo content except the /lib folder who would need to be rewritten, do you confirm ?

@nvuillam
Copy link
Contributor

@admiralAwkbar I was too tempted to resist .... :D

cf: https://github.com/github/super-linter/pull/745/files

GaboFDC
GaboFDC previously approved these changes Sep 21, 2020
Copy link
Collaborator

@GaboFDC GaboFDC left a comment

Choose a reason for hiding this comment

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

This approach looks good to me

lib/linter.sh Show resolved Hide resolved
@admiralAwkbar
Copy link
Collaborator Author

I dont think i like this approach anymore and need to look at this more

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants