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

refactor(archive): move to single-export files #2958

Merged
merged 1 commit into from
Dec 4, 2022
Merged

refactor(archive): move to single-export files #2958

merged 1 commit into from
Dec 4, 2022

Conversation

iuioiua
Copy link
Collaborator

@iuioiua iuioiua commented Nov 29, 2022

This is part of #2936, which aims for leaner dependencies and a more straightforward structure by only having one export per file within the archive module.

@iuioiua iuioiua marked this pull request as ready for review November 29, 2022 21:33
@iuioiua iuioiua requested a review from kt3k as a code owner November 29, 2022 21:33
@iuioiua
Copy link
Collaborator Author

iuioiua commented Nov 29, 2022

Do we want _common.ts and _test_common.ts or _util.ts and _test_util.ts? I'm impartial.

Comment on lines +38 to +40
/** @deprecated (will be removed after 0.169.0) TODO: export all once overlapping objects/types are removed */
export { Tar, type TarData, type TarDataWithSource } from "./tar.ts";
export * from "./untar.ts";
Copy link
Member

Choose a reason for hiding this comment

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

mod.ts doesn't exist now. Does it make sense to add these deprecated versions here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Previously, tar.ts acted as the module entry point. Now that Tar and Untar classes have been separated, mod.ts acts as the entry point for both classes.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It'll also provide a module description where one currently doesn't exist for std/archive.

@kt3k
Copy link
Member

kt3k commented Nov 30, 2022

Do we want _common.ts and _test_common.ts or _util.ts and _test_util.ts? I'm impartial.

hmm both seem having examples:

crypto/_util.ts
fs/_util.ts
media_types/_util.ts
node/path/_util.ts
path/_util.ts

vs

node/_fs/_fs_common.ts
node/_http_common.ts
node/_tls_common.ts
streams/_common.ts
streams/_test_common.ts
uuid/_common.ts

I'm ok with either way.

@iuioiua
Copy link
Collaborator Author

iuioiua commented Nov 30, 2022

Ok, I'll leave it as is then.

Copy link
Member

@kt3k kt3k left a comment

Choose a reason for hiding this comment

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

LGTM

@kt3k kt3k merged commit 259d5a7 into denoland:main Dec 4, 2022
@iuioiua iuioiua deleted the refactor-archive branch December 4, 2022 12:27
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.

None yet

2 participants