🧹 Refactor duplicated logic in applyReservationToAction#45
Merged
max-ostapenko merged 3 commits intomainfrom Mar 5, 2026
Merged
Conversation
- Extracted duplicated logic for prepending statements into `prependStatement` and `isArrayOrString` helpers. - Refactored `applyReservationToAction` to use these helpers for `contextablePreOps`, `contextableQueries`, `proto.preOps`, and `proto.queries`. - Simplified the `action.queries` monkeypatch using `prependStatement`. - Ensured non-mutating behavior for array prepending. - Restored `test-project/workflow_settings.yaml` that was accidentally renamed during matrix tests. Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
Contributor
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
max-ostapenko
deleted the
refactor-applyreservationtoaction-360528593912572714
branch
max-ostapenko
added a commit
that referenced
this pull request
Mar 9, 2026
* refactor: deduplicate logic in applyReservationToAction - Extracted duplicated logic for prepending statements into `prependStatement` and `isArrayOrString` helpers. - Refactored `applyReservationToAction` to use these helpers for `contextablePreOps`, `contextableQueries`, `proto.preOps`, and `proto.queries`. - Simplified the `action.queries` monkeypatch using `prependStatement`. - Ensured non-mutating behavior for array prepending. - Restored `test-project/workflow_settings.yaml` that was accidentally renamed during matrix tests. Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com> * refactor: update exports and enhance test coverage with new utility functions --------- Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
max-ostapenko
added a commit
that referenced
this pull request
Mar 9, 2026
) * Add detection for outer DECLARE statements and update documentation - Implemented `hasOuterDeclare` function to check for outer level DECLARE statements in SQL. - Updated `autoAssignActions` to skip operations with outer DECLARE. - Enhanced README and Copilot instructions to clarify limitations of automated assignment. - Added tests for various scenarios involving outer DECLARE statements. * feat: enhance reservation handling with native support detection * Bump eslint from 10.0.0 to 10.0.1 (#41) Bumps [eslint](https://github.com/eslint/eslint) from 10.0.0 to 10.0.1. - [Release notes](https://github.com/eslint/eslint/releases) - [Commits](eslint/eslint@v10.0.0...v10.0.1) --- updated-dependencies: - dependency-name: eslint dependency-version: 10.0.1 dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump @dataform/core from 3.0.46 to 3.0.47 in /test-project (#42) Bumps [@dataform/core](https://github.com/dataform-co/dataform) from 3.0.46 to 3.0.47. - [Release notes](https://github.com/dataform-co/dataform/releases) - [Commits](dataform-co/dataform@3.0.46...3.0.47) --- updated-dependencies: - dependency-name: "@dataform/core" dependency-version: 3.0.47 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump @dataform/cli from 3.0.46 to 3.0.47 in /test-project (#43) Bumps [@dataform/cli](https://github.com/dataform-co/dataform) from 3.0.46 to 3.0.47. - [Release notes](https://github.com/dataform-co/dataform/releases) - [Commits](dataform-co/dataform@3.0.46...3.0.47) --- updated-dependencies: - dependency-name: "@dataform/cli" dependency-version: 3.0.47 dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Max Ostapenko <1611259+max-ostapenko@users.noreply.github.com> * Bump eslint from 10.0.1 to 10.0.2 (#48) Bumps [eslint](https://github.com/eslint/eslint) from 10.0.1 to 10.0.2. - [Release notes](https://github.com/eslint/eslint/releases) - [Commits](eslint/eslint@v10.0.1...v10.0.2) --- updated-dependencies: - dependency-name: eslint dependency-version: 10.0.2 dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump globals from 17.3.0 to 17.4.0 (#49) Bumps [globals](https://github.com/sindresorhus/globals) from 17.3.0 to 17.4.0. - [Release notes](https://github.com/sindresorhus/globals/releases) - [Commits](sindresorhus/globals@v17.3.0...v17.4.0) --- updated-dependencies: - dependency-name: globals dependency-version: 17.4.0 dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump the npm_and_yarn group across 2 directories with 1 update (#44) Bumps the npm_and_yarn group with 1 update in the / directory: [minimatch](https://github.com/isaacs/minimatch). Bumps the npm_and_yarn group with 1 update in the /test-project directory: [minimatch](https://github.com/isaacs/minimatch). Updates `minimatch` from 3.1.2 to 3.1.5 - [Changelog](https://github.com/isaacs/minimatch/blob/main/changelog.md) - [Commits](isaacs/minimatch@v3.1.2...v3.1.5) Updates `minimatch` from 9.0.5 to 9.0.9 - [Changelog](https://github.com/isaacs/minimatch/blob/main/changelog.md) - [Commits](isaacs/minimatch@v3.1.2...v3.1.5) --- updated-dependencies: - dependency-name: minimatch dependency-version: 3.1.5 dependency-type: indirect dependency-group: npm_and_yarn - dependency-name: minimatch dependency-version: 9.0.9 dependency-type: indirect dependency-group: npm_and_yarn ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Update Dataform version in CI matrix and changelog to 3.0.48 * Refactor isNativeReservationSupported to always return false * Bump eslint from 10.0.0 to 10.0.1 (#41) Bumps [eslint](https://github.com/eslint/eslint) from 10.0.0 to 10.0.1. - [Release notes](https://github.com/eslint/eslint/releases) - [Commits](eslint/eslint@v10.0.0...v10.0.1) --- updated-dependencies: - dependency-name: eslint dependency-version: 10.0.1 dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * 🧹 Refactor duplicated logic in applyReservationToAction (#45) * refactor: deduplicate logic in applyReservationToAction - Extracted duplicated logic for prepending statements into `prependStatement` and `isArrayOrString` helpers. - Refactored `applyReservationToAction` to use these helpers for `contextablePreOps`, `contextableQueries`, `proto.preOps`, and `proto.queries`. - Simplified the `action.queries` monkeypatch using `prependStatement`. - Ensured non-mutating behavior for array prepending. - Restored `test-project/workflow_settings.yaml` that was accidentally renamed during matrix tests. Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com> * refactor: update exports and enhance test coverage with new utility functions --------- Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com> * ⚡ Optimize reservation lookup using Map (#47) * ⚡ Optimize reservation lookup using Map - Refactored `preprocessConfig` to build an optimized `Map` for reservation lookups. - Updated `findReservation` to perform O(1) lookups using the `Map`. - Preserved "first match wins" logic and existing validation. - Maintained backward compatibility by preserving original data structures in preprocessed config. - Improved lookup performance by ~24x in benchmarks. Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com> * Refactor findReservation tests to use Map for action-reservation mapping --------- Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com> * fix badge link * Update changelog for version 0.2.1 * 🧪 [testing improvement] Missing test for compiled objects (proto.preOps) (#46) * 🧪 test: add coverage for compiled objects and proto.preOps - Added comprehensive tests for compiled objects in `test/compiled_objects.test.js`. - Covered `proto.preOps` and `proto.queries` handling for both array and string types. - Verified fallback mechanisms and monkeypatching logic. - Increased line coverage of `index.js` to 100%. Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com> * fix: standardize string quotes in compiled objects tests * refactor: simplify monkeypatched queries tests with parameterized cases --------- Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com> * Remove local integration testing instructions from CONTRIBUTING.md and update isNativeSupported logic in verify_compilation.js * fix createReservationSetter to directly use actionToReservation from preprocessConfig * refactor createReservationSetter * Update CONTRIBUTING.md and README.md for clarity and additional testing instructions * Remove duplicated isArrayOrString and prependStatement functions from index.js * lint --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🎯 What: The code health issue addressed
Refactored duplicated logic in
applyReservationToActioninindex.js.💡 Why: How this improves maintainability
The repeated pattern
if (Array.isArray) ... else if (typeof === 'string') ...was identical across multiple blocks. Extracting this into a helper function improves readability, reduces code size, and ensures consistent behavior (including duplicate checks for both strings and arrays).✅ Verification: How you confirmed the change is safe
npx jestto verify all 29 tests pass.test-project/workflow_settings.yamlwas restored to its correct location.✨ Result: The improvement achieved
Reduced duplication and improved the robustness of the monkeypatch logic.
PR created automatically by Jules for task 360528593912572714 started by @max-ostapenko