-
Notifications
You must be signed in to change notification settings - Fork 25
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
Packaging updates #107
base: main
Are you sure you want to change the base?
Packaging updates #107
Conversation
Can one of the admins verify this patch? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This LGTM - but it would be nice if @colin-grant-work can have a look too please.
This style of export also doesn't play very nicely with the TypeScript language server. In a dependent, I can write an import for the import { MemoryServer } from 'cdt-gdb-vscode'; // Will work import { MemoryServer } from 'cdt-gdb-vscode/dist/extension'; // Will work import { MemoryServer } from 'cdt-gdb-vscode/dist/memory/server/MemoryServer'; // Won't work The problem is that, when the code is bundled, the generated
with one of the last two including the |
@asimgunes please see conversation in #112 also. |
c75b9e4
to
ee38d1d
Compare
Hi @jonahgraham, @colin-grant-work, I rebased the request and keep minimum by only adding required type declerations and minor changes in esbuild code, since @colin-grant-work rollback previous changes. Could you please review the changes again? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks fine to me - but I would appreciate @colin-grant-work's review as he understands this area better.
I'll take a look today |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes look good to me. 👍
Hi @jonahgraham,
I like to discuss this update as a follow-up changes for previous work at pull-request #96.
In this update:
out
folder extract all the output atdist
folder.Benefits:
cdt-gdb-vscode
easy to use as a package depedency.out
folder from the build output (Since entry points already changed todist
folder.)I didnot add any script definition for triggering package publishing operation in npm registry, but, after this update, I believe we can publish cdt-gdb-vscode as an npm package and anyone could easily use cdt-gdb-vscode as a dependent package and extend behaviour.
I hope this would create a positive impact for cdt-gdb-vscode.
Kind regards.
Asim