-
-
Notifications
You must be signed in to change notification settings - Fork 346
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 scripts #588
Cleanup scripts #588
Conversation
…he output was not)
@individual-it : I wanted to add you as a reviewer, but you didn't come up in the drop down for some reason. Would be keen to know what you think. |
I don't think so. We just want to be able run the examples as part of our CI process as a form of feature spec (and of course to make sure they are actually runnable and therefore something people can use). To be honest though, we ought to have a more dedicated feature suite - for another time.
It used to be, it's quite frankly not something I'm too concerned about so I'm happy to see it removed. I think between us we'll be able to judge with PRs etc. if we're badly duplicating code. There is one area of the code base (pact-web) which could be refactored, I don't need jscpd to help me recognise that.
I'm not super concerned about sending the coverage reports to coveralls (although I did think they were still working, not that I've looked recently). The main value is having some guidance around testing and %'s just give us a sense of that. Don't need coveralls to do that for us.
Webpack was needed by the pact-web distribution. As long as that can still build and release then the script step can go. |
BTW the script directory cleanup looks much nicer - thanks heaps for doing it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I cannot see where lint and format are run in CI
I actually would propose to run them directly as first steps in CI, so its very obvious for the developers
Looking at #584 highlighted that it's not super clear when things are triggered with our package.json scripts. I've started to clean up and consolidate them to hopefully make it clearer - thoughts welcome.
I like the guideline that the scripts are named after what they're for, not what they invoke. Like
test
instead ofjest
. Under that guideline, there's still a bit of work to do here.(Matt, I know you don't have a lot of time atm, I'm mostly tagging you in case you're interested)
Things I'm thinking about:
webpack
scriptAll thoughts and comments welcome.