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

Typescript idempotency initial dev branch #1

Merged
merged 20 commits into from
Sep 9, 2022

Conversation

vgphoenixcampos
Copy link
Collaborator

@vgphoenixcampos vgphoenixcampos commented Sep 2, 2022

Description of your changes

How to verify this change

Related issues, RFCs

Issue number:

PR status

Is this ready for review?: NO
Is it a breaking change?: NO

Checklist

  • My changes meet the tenets criteria
  • I have performed a self-review of my own code
  • I have commented my code where necessary, particularly in areas that should be flagged with a TODO, or hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding changes to the examples
  • My changes generate no new warnings
  • The code coverage hasn't decreased
  • I have added tests that prove my change is effective and works
  • New and existing unit tests pass locally and in Github Actions
  • Any dependent changes have been merged and published in downstream module
  • The PR title follows the conventional commit semantics

Breaking change checklist

  • I have documented the migration process
  • I have added, implemented necessary warnings (if it can live side by side)

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

packages/idempotency/tsconfig.json Outdated Show resolved Hide resolved
packages/idempotency/tsconfig.json Show resolved Hide resolved
packages/idempotency/src/PersistenceLayer.ts Outdated Show resolved Hide resolved
packages/idempotency/src/PersistenceLayerInterface.ts Outdated Show resolved Hide resolved
import { PersistenceLayerInterface } from './PersistenceLayerInterface';

abstract class PersistenceLayer implements PersistenceLayerInterface {
public configure(_config: IdempotencyConfig): void {}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I've been distracted so I may have missed the reasoning, but why isn't this the constructor?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We followed what the Python version had, the persistence layer does have its own constructor defined that saves other things on the constructor. I don't have the constructor here currently because many of those objects are related to config that is outside the scope of what we plan to deliver for the first set of functionality

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

import { AnyFunction } from 'types/AnyFunction';
import { IdempotencyOptions } from './IdempotencyOptions';

const makeFunctionIdempotent = (
Copy link
Collaborator

@KevenFuentes9 KevenFuentes9 Sep 8, 2022

Choose a reason for hiding this comment

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

Should we make this a type as well that way we can pass the generic from here down. So we makeFunctionIdempotent<U> and the function would return that Type? Or am I missing something? Otherwise isnt the generic not propagated?

@vgphoenixcampos vgphoenixcampos merged commit c300988 into main Sep 9, 2022
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.

5 participants