Skip to content

feat: add lightning contract action - #1547

Open
doc-han wants to merge 4 commits into
mainfrom
lightning-contract-action
Open

doc-han wants to merge 4 commits into
mainfrom
lightning-contract-action

Conversation

@doc-han

@doc-han doc-han commented Sep 11, 2026

Copy link
Copy Markdown
Collaborator

Short Description

A one or two-sentence description of what this PR does.

Fixes #

Implementation Details

A more detailed breakdown of the changes, including motivations (if not provided in the issue).

QA Notes

List any considerations/cases/advice for testing/QA here.

AI Usage

Please disclose whether you've used AI anywhere in this PR (it's cool, we just
want to know!):

  • I have used Claude Code
  • I have used another model
  • I have not used AI

You can read more details in our
Responsible AI Policy

Release branch checklist

Delete this section if this is not a release PR.

If this IS a release branch:

  • Run pnpm changeset version from root to bump versions
  • Run pnpm install
  • Commit the new version numbers
  • Run pnpm changeset tag to generate tags
  • Push tags git push --tags

Tags may need updating if commits come in after the tags are first generated.

@josephjclark

Copy link
Copy Markdown
Collaborator

@doc-han the action warns of a deprecated node version - can we get that fixed before merging?

jobs:
contract:
name: test build against lightning
if: contains(github.event.pull_request.labels.*.name, 'run_contract_tests')

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

So the tests only run on PRs with the label right now

Worried about that - if users forget to add the label, the tests will never run. In practice that means they'll never run.

I'd rather have tests run on all Ready PRs with a reviewer, so that hopefully we always run integration tests just before merging

Same idea on lightning too

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

resolved. label dependency removed

runs-on: ubuntu-latest
timeout-minutes: 45
services:
postgres:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

can we not default all this stuff? Does it really need to be declared in the action?

@@ -0,0 +1,22 @@
name: Integration Testing

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'd like to update the filename here too so that it maps to the actual action name. Otherwise it's hard to find

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

resolved

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: New Issues

Development

Successfully merging this pull request may close these issues.

3 participants