Skip to content

Conversation

leoromanovsky
Copy link
Member

@leoromanovsky leoromanovsky commented Aug 29, 2023


labels: mergeable

Fixes: #issue

Motivation and Context

Noticed that both yarn.lock and package-lock.json files are present; yarn looked more broadly used so I decided to stick to it.

Description

  • remove usage of npm
  • make pre-commit executable
  • fix pre commit hook by adding lint-staged dev package

How has this been tested?

@leoromanovsky
Copy link
Member Author

Test failure:

 FAIL  src/client/eppo-client.spec.ts
  ● Console

    console.error
      [Eppo SDK] Error logging assignment event: logging error

      149 |       this.assignmentLogger.logAssignment(event);
      150 |     } catch (error) {
    > 151 |       console.error(`[Eppo SDK] Error logging assignment event: ${error.message}`);
          |               ^
      152 |     }
      153 |   }
      154 |

      at EppoClient.logAssignment (src/client/eppo-client.ts:151:15)
      at EppoClient.getAssignment (src/client/eppo-client.ts:92:10)
      at Object.<anonymous> (src/client/eppo-client.spec.ts:249:31)

  ● EppoClient E2E test › getAssignment › test variation assignment splits

    expect(received).toEqual(expected) // deep equality

    - Expected  - 4
    + Received  + 4

      Array [
    -   10,
    -   10,
    -   10,
    -   5,
    +   null,
    +   null,
    +   null,
    +   null,
      ]

      181 |           ? getAssignmentsWithSubjectAttributes(subjectsWithAttributes, experiment)
      182 |           : getAssignments(subjects, experiment);
    > 183 |         expect(assignments).toEqual(expectedAssignments);
          |                             ^
      184 |       },
      185 |     );
      186 |   });

      at src/client/eppo-client.spec.ts:183:29

Copy link

@chasdevs chasdevs left a comment

Choose a reason for hiding this comment

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

LGTM but what's with all the docs changes?

@leoromanovsky
Copy link
Member Author

LGTM but what's with all the docs changes?

the pre-commit hook was broken until now (not executable) and there must have been a back-log of changes. personally I don't get the purpose of these docs and think I will just delete them.

@leoromanovsky leoromanovsky merged commit 4fdcce5 into main Aug 29, 2023
leoromanovsky added a commit that referenced this pull request Aug 30, 2023
* remove npm; use yarn

* fix precommit hook and add lint-staged as a dev dep

* fix precommit hook and add lint-staged as a dev dep

* use node:18 as github runner

* remove docs and api builder

* temporarily cast expected assignments to string
leoromanovsky added a commit that referenced this pull request Aug 31, 2023
* remove npm; use yarn

* fix precommit hook and add lint-staged as a dev dep

* fix precommit hook and add lint-staged as a dev dep

* use node:18 as github runner

* remove docs and api builder

* temporarily cast expected assignments to string
leoromanovsky added a commit that referenced this pull request Aug 31, 2023
* remove npm; use yarn

* fix precommit hook and add lint-staged as a dev dep

* fix precommit hook and add lint-staged as a dev dep

* use node:18 as github runner

* remove docs and api builder

* temporarily cast expected assignments to string
leoromanovsky added a commit that referenced this pull request Aug 31, 2023
* remove npm; use yarn

* fix precommit hook and add lint-staged as a dev dep

* fix precommit hook and add lint-staged as a dev dep

* use node:18 as github runner

* remove docs and api builder

* temporarily cast expected assignments to string
@typotter typotter deleted the lr/use-yarn branch March 12, 2025 16:19
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