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.

Sign up for GitHub

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

Jump to bottom

Update the abstraction library code to use import type for interfaces #789

Merged
merged 19 commits into from
Oct 12, 2023
Merged

Update the abstraction library code to use import type for interfaces #789

merged 19 commits into from
Oct 12, 2023

Conversation

koros
Copy link
Contributor

@koros koros commented Aug 14, 2023

Use import type whenever referencing the folowing interfaces:

interface ApiError 
interface RequestAdapter 
interface RequestOption
interface ResponseHandler
interface AccessTokenProvider
interface AuthenticationProvider
interface AdditionalDataHolder
interface Parsable
interface ParseNode
interface ParseNodeFactory
interface SerializationWriter
interface SerializationWriterFactory
interface BackedModel
interface BackingStore
interface BackingStoreFactory
interface ObservabilityOptions
interface Middleware 
interface ChaosHandlerOptions
interface TelemetryHandlerOptions
interface TestEntity
interface TestParser

@koros koros temporarily deployed to build_test August 14, 2023 16:05 — with GitHub Actions Inactive
@koros koros temporarily deployed to build_test August 14, 2023 16:05 — with GitHub Actions Inactive
baywet
baywet reviewed Aug 14, 2023
View reviewed changes
Copy link
Member

@baywet baywet left a comment

Choose a reason for hiding this comment

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

We might want to setup an eslint rule to make sure we didn't forget anything and won't have any regression in the future. https://typescript-eslint.io/rules/consistent-type-imports/

@baywet baywet temporarily deployed to build_test August 24, 2023 15:49 — with GitHub Actions Inactive
@baywet baywet temporarily deployed to build_test August 24, 2023 15:49 — with GitHub Actions Inactive
@baywet baywet temporarily deployed to build_test August 24, 2023 15:49 — with GitHub Actions Inactive
baywet
baywet previously approved these changes Aug 25, 2023
View reviewed changes
@baywet
Copy link
Member

baywet commented Aug 25, 2023

@koros can we merge that or are you still working on it? I'd like to release today if possible to get #790 and #805 by @jimmywim
out as soon as possible. If you still need to work on this, I'll release just the community PR then.

@sebastienlevert
Copy link

Ping @koros

@baywet
Copy link
Member

baywet commented Aug 28, 2023

My question was replied offline, still more work required on this one, and it might take a while. I'll go ahead and create the tags so we can at list have the community fix out.

@koros
Copy link
Contributor Author

koros commented Aug 28, 2023

The eslint rule flags a lot more other instances which I have put some fix on the generation side. Let me do a final pass on this.

@koros koros temporarily deployed to build_test August 29, 2023 08:43 — with GitHub Actions Inactive
@koros koros temporarily deployed to build_test August 29, 2023 08:43 — with GitHub Actions Inactive
@koros koros temporarily deployed to build_test August 29, 2023 08:43 — with GitHub Actions Inactive
@koros koros temporarily deployed to build_test September 26, 2023 08:06 — with GitHub Actions Inactive
@koros koros temporarily deployed to build_test September 26, 2023 08:06 — with GitHub Actions Inactive
@sebastienlevert
Copy link

Is it me or it's been sitting here for a long time? @baywet or @koros?

@koros koros temporarily deployed to build_test October 6, 2023 10:02 — with GitHub Actions Inactive
@koros koros temporarily deployed to build_test October 6, 2023 10:02 — with GitHub Actions Inactive
@koros koros temporarily deployed to build_test October 6, 2023 20:02 — with GitHub Actions Inactive
@koros koros temporarily deployed to build_test October 6, 2023 20:02 — with GitHub Actions Inactive
@baywet baywet temporarily deployed to build_test October 10, 2023 14:42 — with GitHub Actions Inactive
@baywet baywet temporarily deployed to build_test October 10, 2023 14:42 — with GitHub Actions Inactive
@baywet baywet temporarily deployed to build_test October 10, 2023 14:42 — with GitHub Actions Inactive
@github-actions
Copy link
Contributor

Generated "test-kiota-typescript-libraries" SDK package:

File count Size (Kb) Unpacked (Kb)
87599 9968.86 182151.04

@github-actions
Copy link
Contributor

Generated sample app bundle:

Stat Size (Kb) Parsed Size (Kb) Gzipped (Kb)
45938.23 11339.82 587.83

@koros koros merged commit aba545b into main Oct 12, 2023
7 checks passed
@koros koros deleted the feature/use-import-type-for-interfaces branch October 12, 2023 12:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@baywet baywet baywet approved these changes

@darrelmiller darrelmiller Awaiting requested review from darrelmiller

@zengin zengin Awaiting requested review from zengin

@MIchaelMainer MIchaelMainer Awaiting requested review from MIchaelMainer

@ddyett ddyett Awaiting requested review from ddyett

@gavinbarron gavinbarron Awaiting requested review from gavinbarron

@andrueastman andrueastman Awaiting requested review from andrueastman

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@koros @baywet @sebastienlevert