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

feat: Typescript 5/4 #72

Merged
merged 6 commits into from
Feb 9, 2023
Merged

feat: Typescript 5/4 #72

merged 6 commits into from
Feb 9, 2023

Conversation

jimmycallin
Copy link
Contributor

  • I have added automatic tests where applicable
  • The PR title is suitable as a release note
  • The PR contains a description of what has been changed
  • The description contains manual test instructions

Changes

Additional fixes and enhancements from trying out the beta.

  • Exposing all interfaces. I did this by exporting everything (and removing default exports). I don't think this should break anything, but look over the operation changes in particular.
  • Added typing to the metadata object in the Response interface

Test

@jimmycallin jimmycallin requested a review from a team as a code owner February 7, 2023 22:52
source/operation.ts Outdated Show resolved Hide resolved
@@ -81,7 +72,7 @@ export type Operation =
* @param {Object} data Entity data to use for creation
* @return {Object} API operation
*/
export function createOperation(type: string, data: any): CreateOperation {
export function create(type: string, data: any): CreateOperation {
Copy link
Contributor

Choose a reason for hiding this comment

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

Even though they probably are not ment to be used outside this project, must we not wait with changing function names until it's time to make a major version release?

Copy link
Contributor Author

@jimmycallin jimmycallin Feb 8, 2023

Choose a reason for hiding this comment

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

they were not exposed publicly from index.ts, so only way to have used them would be if someone imported them directly from @ftrack/api/dist/operation, which I don't think is supported behavior. The function names reflect their names in the now removed default object.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think I'm just overly cautious.

@jimmycallin jimmycallin merged commit b0f85aa into main Feb 9, 2023
@jimmycallin jimmycallin deleted the typescript-5 branch February 9, 2023 09:32
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.

3 participants