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

Release 0.9.0 #27

Closed
wants to merge 6 commits into from
Closed

Release 0.9.0 #27

wants to merge 6 commits into from

Conversation

westbury
Copy link

This is a PR of the new Mercurial extension into a temporary branch so it can be reviewed. There is also a second commit that is a copy of the PR put into Theia. See eclipse-theia#6426 .

@westbury westbury force-pushed the release-0.9.0 branch 2 times, most recently from 5d5735c to 384222b Compare October 23, 2019 16:17
Copy link

@mcgordonite mcgordonite left a comment

Choose a reason for hiding this comment

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

Nice to finally have an extension for Mercurial!

There's quite a lot of opportunity for code re-use between the git and hg extensions, but I don't think it's worth going much further until we know what Theia's SCM strategy will be.

Comments are just for low-handing fruit that I think will be quick to fix.

packages/hg/README.md Outdated Show resolved Hide resolved
packages/hg/src/node/init/hg-init.ts Outdated Show resolved Hide resolved
packages/hg/src/node/init/hg-init.ts Outdated Show resolved Hide resolved
packages/hg/package.json Outdated Show resolved Hide resolved
packages/hg/src/node/hg/hg-command-server.ts Outdated Show resolved Hide resolved

// This implementation leaves all command servers running. There is no mechanism for shutting
// down command servers for repoistories that the user is no longer interacting with.
const commandServers: Map<string, HGRepo> = new Map();

Choose a reason for hiding this comment

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

Rather than storing state in the module, it would be better to add an injectable class that tracks the command servers.

packages/hg/src/node/hg/HGRepo.ts Outdated Show resolved Hide resolved
packages/hg/src/node/test/binding-helper.ts Outdated Show resolved Hide resolved
packages/hg/src/node/test/no-sync-repository-manager.ts Outdated Show resolved Hide resolved
@0Grit
Copy link

0Grit commented Oct 25, 2019

What are the teams plans for mercurial moving forward?
Personally the legacy repos and Mbed "Code" site make for a disjointed experience.
Not to mention and added dependency for mbed-cli.

Copy link

@mcgordonite mcgordonite left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for updating.

westbury and others added 4 commits October 30, 2019 11:08
Signed-off-by: Nigel Westbury <[email protected]>

Clean up css class names and other code review changes
The version of TypeScript and @types/react used in mbs-ide are not
compatible with the `event` type replaced by this commit.

This commit can be reverted from ARMmbed/theia when:
- Theia updates TS or @types/react such that the original code compiles
- mbs-ide no longer uses @theia/scm in its workspace.
Shut down Mercurial command servers after 10 seconds of inactivity

Clean out unneeded code

Added test for HgCommandServer utils.
@westbury westbury closed this Oct 31, 2019
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