Skip to content

suites#157

Merged
thedavidmeister merged 3 commits intomainfrom
2025-10-11-deploy
Oct 11, 2025
Merged

suites#157
thedavidmeister merged 3 commits intomainfrom
2025-10-11-deploy

Conversation

@thedavidmeister
Copy link
Copy Markdown
Contributor

@thedavidmeister thedavidmeister commented Oct 11, 2025

Motivation

Solution

Checks

By submitting this for review, I'm confirming I've done the following:

  • made this PR as small as possible
  • unit-tested any new functionality
  • linked any relevant issues or PRs
  • included screenshots (if this involves a front-end change)

Summary by CodeRabbit

  • New Features
    • Added selectable deployment suites via DEPLOYMENT_SUITE (options: all, tables, contract); defaults to all.
    • Manual workflow now accepts a "suite" choice so manual artifact runs can target a specific deployment suite.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Oct 11, 2025

Walkthrough

Adds deployment-suite constants and environment-based selection to script/Deploy.sol, computing a keccak256 suite selector and conditionally running table and contract deployment steps. Also adds a required suite workflow input to .github/workflows/manual-sol-artifacts.yaml to control which suite runs.

Changes

Cohort / File(s) Summary
Deployment suite gating
script/Deploy.sol
Added top-level constants DEPLOYMENT_SUITE_ALL, DEPLOYMENT_SUITE_TABLES, DEPLOYMENT_SUITE_CONTRACT; read DEPLOYMENT_SUITE from environment (default "all"), compute keccak256 as suite, and conditionally call container.writeZoltu() and LibDecimalFloatDeploy.decimalFloatZoltu() based on suite membership (replacing unconditional calls).
Workflow input
.github/workflows/manual-sol-artifacts.yaml
Added a workflow_dispatch input named suite (required choice) with options all, deployment.suite.tables, deployment.suite.contract to allow manual selection of the deployment suite.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Env as ENV
  participant Deploy as Deploy.sol
  participant Tables as container.writeZoltu()
  participant Contract as LibDecimalFloatDeploy.decimalFloatZoltu()

  Note over Deploy: Initialization
  Env->>Deploy: DEPLOYMENT_SUITE (default "all")
  Deploy->>Deploy: suite = keccak256(DEPLOYMENT_SUITE)

  alt suite == ALL or TABLES
    Deploy->>Tables: writeZoltu()
    Tables-->>Deploy: done
  else
    Note over Deploy: Skip tables
  end

  alt suite == ALL or CONTRACT
    Deploy->>Contract: decimalFloatZoltu()
    Contract-->>Deploy: done
  else
    Note over Deploy: Skip contract
  end

  Note over Deploy: Deployment complete
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title Check ❓ Inconclusive The title “suites” is overly generic and does not convey the primary change of introducing deployment suite constants and conditional deployment logic in Deploy.sol, nor the addition of a “suite” input to the GitHub Actions workflow. It fails to provide sufficient context for reviewers or future maintainers to understand the purpose of the PR at a glance. Because it uses a non-descriptive term, it is unclear what aspect of “suites” is being modified. Please revise the title to clearly summarize the main changes, for example “Add deployment suite constants and conditional logic in Deploy.sol and workflow_dispatch input.” A more specific title will help reviewers and future maintainers understand the PR intent at a glance.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch 2025-10-11-deploy

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 78e15cb and 164435c.

📒 Files selected for processing (1)
  • script/Deploy.sol (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: rainix (macos-latest, rainix-rs-test)
  • GitHub Check: rainix (macos-latest, rainix-sol-legal)
🔇 Additional comments (1)
script/Deploy.sol (1)

25-31: Conditional logic structure is correct.

The conditional deployment logic correctly handles selective execution:

  • Tables deploy when suite is ALL or TABLES
  • Contract deploys when suite is ALL or CONTRACT
  • The "ALL" case properly triggers both deployments

However, this logic inherits the critical constant mismatch issue flagged above. Once the constants are fixed to match expected user inputs and validation is added, this implementation will work correctly.

Comment thread script/Deploy.sol
Comment thread script/Deploy.sol
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 164435c and 23ba3d5.

📒 Files selected for processing (1)
  • .github/workflows/manual-sol-artifacts.yaml (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#157
File: script/Deploy.sol:9-11
Timestamp: 2025-10-11T14:36:29.947Z
Learning: In script/Deploy.sol for the rain.math.float repository, the DEPLOYMENT_SUITE constants use full qualified dotted notation (e.g., "deployment.suite.tables", "deployment.suite.contract") intentionally. Users select deployment options by setting the DEPLOYMENT_SUITE environment variable to these exact strings, not shortened versions like "tables" or "contract".
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
  • GitHub Check: rainix (ubuntu-latest, test-wasm-build)
  • GitHub Check: rainix (ubuntu-latest, rainix-sol-test)
  • GitHub Check: rainix (ubuntu-latest, rainix-rs-static)
  • GitHub Check: rainix (ubuntu-latest, rainix-sol-static)
  • GitHub Check: rainix (macos-latest, rainix-sol-legal)
  • GitHub Check: rainix (macos-latest, rainix-rs-test)
  • GitHub Check: rainix (ubuntu-latest, rainix-rs-test)
  • GitHub Check: rainix (ubuntu-latest, rainix-sol-legal)
  • GitHub Check: git-clean

Comment thread .github/workflows/manual-sol-artifacts.yaml
@thedavidmeister thedavidmeister merged commit 4473754 into main Oct 11, 2025
9 checks passed
@github-actions
Copy link
Copy Markdown

@coderabbitai assess this PR size classification for the totality of the PR with the following criterias and report it in your comment:

S/M/L PR Classification Guidelines:

This guide helps classify merged pull requests by effort and complexity rather than just line count. The goal is to assess the difficulty and scope of changes after they have been completed.

Small (S)

Characteristics:

  • Simple bug fixes, typos, or minor refactoring
  • Single-purpose changes affecting 1-2 files
  • Documentation updates
  • Configuration tweaks
  • Changes that require minimal context to review

Review Effort: Would have taken 5-10 minutes

Examples:

  • Fix typo in variable name
  • Update README with new instructions
  • Adjust configuration values
  • Simple one-line bug fixes
  • Import statement cleanup

Medium (M)

Characteristics:

  • Feature additions or enhancements
  • Refactoring that touches multiple files but maintains existing behavior
  • Breaking changes with backward compatibility
  • Changes requiring some domain knowledge to review

Review Effort: Would have taken 15-30 minutes

Examples:

  • Add new feature or component
  • Refactor common utility functions
  • Update dependencies with minor breaking changes
  • Add new component with tests
  • Performance optimizations
  • More complex bug fixes

Large (L)

Characteristics:

  • Major feature implementations
  • Breaking changes or API redesigns
  • Complex refactoring across multiple modules
  • New architectural patterns or significant design changes
  • Changes requiring deep context and multiple review rounds

Review Effort: Would have taken 45+ minutes

Examples:

  • Complete new feature with frontend/backend changes
  • Protocol upgrades or breaking changes
  • Major architectural refactoring
  • Framework or technology upgrades

Additional Factors to Consider

When deciding between sizes, also consider:

  • Test coverage impact: More comprehensive test changes lean toward larger classification
  • Risk level: Changes to critical systems bump up a size category
  • Team familiarity: Novel patterns or technologies increase complexity

Notes:

  • the assessment must be for the totality of the PR, that means comparing the base branch to the last commit of the PR
  • the assessment output must be exactly one of: S, M or L (single-line comment) in format of: SIZE={S/M/L}
  • do not include any additional text, only the size classification
  • your assessment comment must not include tips or additional sections
  • do NOT tag me or anyone else on your comment

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Oct 11, 2025

Tip

For best results, initiate chat on the files or code changes.

SIZE=M

@coderabbitai coderabbitai Bot mentioned this pull request Feb 6, 2026
4 tasks
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.

1 participant