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

chore(ci): add separate github action workflow that runs eslint #5434

Merged
merged 1 commit into from
Oct 22, 2024

Conversation

SamMousa
Copy link
Contributor

The current azure pipelines are very hard to read for developers not familiar with them.
Also they are very slow and do a lot.

This PR adds an extra workflow for PRs that just runs eslint via github actions. Note: since this is an open source repo the CI minutes for this are free afaik.

@SamMousa
Copy link
Contributor Author

Just check this ESLint speed xD 58 seconds!!

@tsv2013
Copy link
Member

tsv2013 commented Oct 18, 2024

@SamMousa We highly appreciate your contributions and valuable feedback. I'm so sorry for our late responses. We should react on PRs immediately but sometimes haven't enough time.

As for this PR - it's great, but we're using Azure devops for the full production lifecycle: from changes to core library to the libraries release, npm packages publishing and updating our site. That's why GH actions can be only "just for inormation" for us and willn't block the release in case of issues. We don't want to have several pipelines at the same time - it will make release process more complex. That's why I'd prefer to refuse this PR.

@tsv2013 tsv2013 closed this Oct 18, 2024
@SamMousa
Copy link
Contributor Author

I'm so sorry for our late responses. We should react on PRs immediately but sometimes haven't enough time.

No worries, it happens. When it's a feature I really need I'll push for it otherwise I just let it rest till you have time!

As for this PR - it's great, but we're using Azure devops for the full production lifecycle: from changes to core library to the libraries release, npm packages publishing and updating our site.

I think this is a bad choice for outside contributors; it makes it really hard to understand why things fail. But that said, it is your choice and you should not change it just for me.

That's why GH actions can be only "just for inormation" for us and willn't block the release in case of issues.

You can mark the job as optional so a failure won't block a release. Or you let it run only on PRs from forks not from the main repo. My goal here is to make it easier for people to make good PRs that you can actually review for its contents instead of having to extract errors from the depths of azure manually so people can fix their PR.

In conclusion I believe that there's a significant barrier stopping many of your users / customers from contributing because of the toolchain you use. What you do with that is up to you!

@tsv2013
Copy link
Member

tsv2013 commented Oct 18, 2024

I think this is a bad choice for outside contributors; it makes it really hard to understand why things fail.
Don't you have access to public pipelines like https://dev.azure.com/SurveyJS/SurveyJS%20Integration%20Tests/_build ?
This is the only place where we're keeping eyes on your actual build status.

You can mark the job as optional so a failure won't block a release.
To be honest right now i think we could add the workfow you proposed to our github actions. Nothing bad happens. I can reactivate PR and merge it. This pipeline will be optional build status indicator because it duplicates one of azure build steps.

The main thing I think is that right now nobody except me and you will see and check this indicator.

In conclusion I believe that there's a significant barrier stopping many of your users / customers from contributing because of the toolchain you use.
I can't agree with you here - GitHub yaml IMO not much better that Azure yaml. It's just another one yaml :-)
But actually we need to declare clear code of conduct for our repos:

  • implement a new feature for all frameworks we're supporting (react, angular, vue and so on)
  • add tests (unit or functional) for proposed code
  • pass lint checks
  • may be something else

@tsv2013 tsv2013 reopened this Oct 18, 2024
@tsv2013 tsv2013 self-requested a review October 18, 2024 12:52
@tsv2013 tsv2013 self-assigned this Oct 18, 2024
@SamMousa
Copy link
Contributor Author

I can't agree with you here - GitHub yaml IMO not much better that Azure yaml. It's just another one yaml :-)

The configuration is not my argument, it's the extraction of results, maybe this can be improved even from azure pipelines. Also there's like a social network pressure; if the de facto standard is github actions you'll have more people with knowledge on that.

image

That error is not very informative and I have to follow links, which in the example above don't even work xD

By the way, don't feel forced to merge this, just create some code of conduct, I'm fine with adapting to another process, just hard to do if I don't know what it is.

@tsv2013
Copy link
Member

tsv2013 commented Oct 18, 2024

image
You can click the "View more details on Azure Pipelines" link and full job report report should be available. Can you check whwther it is accessible for you?

just hard to do if I don't know what it is.
You are absolutely right indeed.

@SamMousa
Copy link
Contributor Author

Can you check whwther it is accessible for you?

https://dev.azure.com/SurveyJS/SurveyJS%20Integration%20Tests/_build/results?buildId=107052&view=logs&jobId=c87dbe1d-c2b4-54bc-5c60-4be654e9a189

That one isn't, but could be an old log that got purged. In general at least some of them are accessible, since I've been forced into the detailed logs over there before ;-)

@tsv2013 tsv2013 merged commit 3acc052 into surveyjs:master Oct 22, 2024
9 of 12 checks passed
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