-
Notifications
You must be signed in to change notification settings - Fork 1
Conversation
That's funny, it even tested itself before the merge. Wasn't expecting that. I guess I could have done it through a PR. This exemplifies my claims. You can see the I can rebase #27 here once done to avoid merge a failing CI to the trunk. |
Yes, this also happens for #33. How these PRs differ apart from linting? Other than that, seems legit. |
For starters, this one works. #33 fails to install dependencies. This one uses yarn instead of npm, the same we use to develop the application, uses
Linting errors shouldn't be ignored. Even when they are accepted by the remote (pre-commit hook shouldn't allow), I think a CI fail tag to become clear that this specific version is failing linting could be very handy. Also, if one is having trouble with linting errors, or a way of using a tool doesn't meet out styling principles, ignore directives are a thing. |
I don't think there are benefits for running linters on CI, in contrast to running tests, for example. All this is gonna do is activate bad neurons with all the ❌es |
All neurons are bad neurons. But sure, I can just remove the lint job |
Can you merge these yamls into #33? I have not taken care of errors there because we didn't even have tests, but we'll want code coverage. |
Just taking the Codecov job should work, but we need to know the parameter to set to point to server and front separately |
Obviously depends on #27, tests action currently fail on my fork, but it is similar enough to the linting action to guarantee it will work, once testing is in place.
I did on a separate fork because it is a lot of actions CI trial-and-error, and did not want to commit-spam. It was the easiest proven working method to try stuff with GitHub CI. So, as to avoid commit-spam, we should squash this before merging.