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

GitHub Actions #97

Closed

Conversation

peterbecich
Copy link
Contributor

@peterbecich peterbecich commented Feb 15, 2023

This is a GitHub Action for Cabal using https://github.com/haskell/actions/tree/main/setup#usage
The PR includes cabal.project to test all three projects

Test: #97 (comment)

Cabal build appears to be broken on master right now. I think it has something to do with https://hackage.haskell.org/package/hoauth2-2.6.0 Stack build succeeds. Please merge this and I will fix the Cabal build later.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Feb 15, 2023
@k8s-ci-robot
Copy link
Contributor

Welcome @peterbecich!

It looks like this is your first PR to kubernetes-client/haskell 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-client/haskell has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

@k8s-ci-robot k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Feb 15, 2023
@k8s-ci-robot
Copy link
Contributor

Hi @peterbecich. Thanks for your PR.

I'm waiting for a kubernetes-client member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Feb 15, 2023
@thomasjm
Copy link
Contributor

Cool! I'd still suggest converting this to the Haskell setup actions -- the one I linked in the other discussion shows how to concisely test both Cabal and Stack.

@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 6, 2023
@peterbecich peterbecich force-pushed the github-actions branch 2 times, most recently from 3ce7569 to 462083c Compare March 6, 2023 04:29
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Mar 6, 2023
@thomasjm
Copy link
Contributor

thomasjm commented Mar 8, 2023

Let me know when this is ready for review! For now I'd suggest adding a few different Stack LTSes.

@peterbecich
Copy link
Contributor Author

@thomasjm yes, please review. I am trying to focus on the Cabal build. It appears to be failing on master right now. Having this new GitHub Action would be useful to fixing the Cabal build later, IMO. I don't have time to fix it right now in this PR.

@jonschoning
Copy link
Contributor

jonschoning commented Mar 9, 2023

I tried building locally with cabal & ghc 9.4.4 and it couldn't satisfy all the constraints, perhaps we should exclude 9.4.4 from the matrix?

Also, anything in kubernetes/ is going to get auto-generated and overwritten; the only thing we usually change is in kubernetes/kubernetes-client-core.cabal apart from running autogen is the package version. Any other changes in kubernetes/ should be upstreamed to https://github.com/OpenAPITools/openapi-generator/ so that the appropriate output is generated

kubernetes-client/package.yaml Outdated Show resolved Hide resolved
.github/workflows/stack.yml Outdated Show resolved Hide resolved
.github/workflows/stack.yml Outdated Show resolved Hide resolved
peterbecich and others added 3 commits March 11, 2023 16:58
Co-authored-by: Tom McLaughlin <pyro777@gmail.com>
Co-authored-by: Tom McLaughlin <pyro777@gmail.com>
@peterbecich
Copy link
Contributor Author

Thanks @jonschoning and @thomasjm

Can we keep 9.4.4 in the build matrix, despite the failure, so that it exposes the issue and someone else might fix it? I will try to fix it myself, too.

I've removed the change from kubernetes/

@peterbecich peterbecich requested review from thomasjm and removed request for jonschoning and brendandburns March 12, 2023 01:10
Copy link
Contributor

@thomasjm thomasjm left a comment

Choose a reason for hiding this comment

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

Just a couple formatting nits. Otherwise if the tests are running then LGTM!

fail-fast: false
matrix:
ghc:
- "8.10.7"
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: dedent these to be consistent with other list items in this file

Comment on lines 8 to 10
ghc: ['8.10.7', '9.0.2', '9.2.7', '9.4.4']
cabal: ['3.8.1.0']
os: [ubuntu-latest]
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: convert these to YAML-style lists (with -) to be consistent with the Stack file?

Copy link
Contributor

@thomasjm thomasjm left a comment

Choose a reason for hiding this comment

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

Oh, I'd also suggest just combining the Cabal and Stack workflows into a single workflow file. That way you can see all the results in one page of GitHub Actions.

.github/workflows/stack.yml Outdated Show resolved Hide resolved
peterbecich and others added 2 commits March 11, 2023 19:32
Co-authored-by: Tom McLaughlin <pyro777@gmail.com>
@peterbecich
Copy link
Contributor Author

Good idea to combine the files. The tests are running: https://github.com/peterbecich/haskell-kubernetes-client/actions/runs/4395599081

The GHC 9.4 failure persists, I will try to fix that here: #101

@thomasjm
Copy link
Contributor

Nice to see the Stack tests passing! It looks like all the Cabal ones are failing though? Do we need to pin the version of hoauth2 better in the Cabal case?

@thomasjm
Copy link
Contributor

Just checking in here @peterbecich -- it looks like there was some progress on the hoauth2 situation mentioned in #101. Any chance this will fix the Cabal builds? It would be cool to get this (and #101) merged.

Btw: GHC 9.4.5 is out and now on Stackage nightly, so maybe we should update these PRs?

Copy link
Contributor

@thomasjm thomasjm left a comment

Choose a reason for hiding this comment

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

Please update 9.4.4 -> 9.4.5 in ci.yml

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: peterbecich
Once this PR has been reviewed and has the lgtm label, please assign akshaymankar for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@thomasjm
Copy link
Contributor

@peterbecich just checking in, any chance we can push this over the finish line?

@thomasjm
Copy link
Contributor

FWIW, I took your branch and made some more progress. Branch here:

https://github.com/codedownio/kubernetes-client-haskell/tree/github-actions

I'm stuck on an issue with jsonpath-0.3.0.0, filed a help request here:

akshaymankar/jsonpath-hs#37

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 5, 2023
@k8s-ci-robot
Copy link
Contributor

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@thomasjm
Copy link
Contributor

thomasjm commented Jul 5, 2023

This can be closed now with #104 landed.

@jonschoning
Copy link
Contributor

/close

@k8s-ci-robot
Copy link
Contributor

@jonschoning: Closed this PR.

In response to this:

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants