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

ci: Add workflow file for pull requests #59

Merged

Conversation

abhishek-exists
Copy link
Contributor

@abhishek-exists abhishek-exists commented Sep 10, 2023

Adds ci file which checks lint & formatting along with build for pull requests & direct push

@chrisvxd Let me know if there are any changes you want

@vercel
Copy link

vercel bot commented Sep 10, 2023

@abhishek276533 is attempting to deploy a commit to the Measured Team on Vercel.

A member of the Team first needs to authorize it.

Copy link
Member

@chrisvxd chrisvxd left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution @abhishek276533! Very necessary PR.

Unfortunately the build failed - please see my comment.

.github/workflows/ci.yml Outdated Show resolved Hide resolved
@chrisvxd
Copy link
Member

I just realised this issue is marked as fixing #48, but that's not quite right. This PR adds linting / tests, but doesn't generate and run an app (per #48). That doesn't make this PR any less valid, but we shouldn't close #48.

Copy link
Member

@chrisvxd chrisvxd left a comment

Choose a reason for hiding this comment

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

Nice, nearly there now! 👍

Build is failing due to issue with generator step - please see comment.

.github/workflows/ci.yml Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
@abhishek-exists
Copy link
Contributor Author

abhishek-exists commented Sep 11, 2023

@chrisvxd I'm facing the same error while running yarn build inside recipes/next in my local as well.

@chrisvxd
Copy link
Member

@abhishek276533 Hmm, I can't reproduce but I think let's just remove the final "Install dependencies and Build generated code" build step for now and address #48 separately, along with #10. This at least gets the CI in.

@abhishek-exists
Copy link
Contributor Author

@chrisvxd Have you tried running yarn build inside the recipes/next directory?
I've added the requested changes.

Copy link
Member

@chrisvxd chrisvxd left a comment

Choose a reason for hiding this comment

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

Working now! Thanks @abhishek276533.

Yes I tested yarn build in the recipe and it worked. It might be my local environment but let's discuss in #48.

@chrisvxd chrisvxd merged commit e1152d2 into measuredco:main Sep 11, 2023
1 check 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