-
Notifications
You must be signed in to change notification settings - Fork 205
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
Rename daml2ts to daml2js #5394
Conversation
This is pretty much a search and replace over the whole repo, including file names. CHANGELOG_BEGIN CHANGELOG_END
1f02611
to
c763dd3
Compare
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.
Looks very reasonable.
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.
LGTM apart from the fact that I find it confusing that we call it daml2js
and then users only interact with it by calling daml codegen js
. But that’s new to this PR and I’m happy to leave it up to you.
runDaml2js remainingArgs = do | ||
daml2js <- fmap (</> "daml2js" </> "daml2js") getSdkPath | ||
withProcessWait_' (proc daml2js remainingArgs) (const $ pure ()) `catchIO` | ||
(\e -> hPutStrLn stderr "Failed to invoke daml2js." *> throwIO e) |
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.
Shouldn’t this say daml codegen js
. The executable invoked by that seems like an implementation detail.
@@ -1,25 +1,25 @@ | |||
.. Copyright (c) 2020 Digital Asset (Switzerland) GmbH and/or its affiliates. All rights reserved. | |||
.. SPDX-License-Identifier: Apache-2.0 | |||
|
|||
``daml2ts`` - TypeScript code generator for DAML | |||
``daml2js`` - TypeScript code generator for DAML |
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.
Somewhat similar question. I think it’s slightly confusing to have this called daml2js
while the command is called daml codegen js
.
I agree regarding the docs. I'll have a walk through the docs today and look at all occurrences of |
@hurryabit @Robin-da Note that we need to update the architecture diagrams then as well as they're pointing to daml2ts at the moment. |
This is pretty much a search and replace over the whole repo, including
file names.
This does not yet add the
daml codegen js
command. We'll do thatin a separate PR.
CHANGELOG_BEGIN
CHANGELOG_END
Pull Request Checklist
CHANGELOG_BEGIN
andCHANGELOG_END
tagsNOTE: CI is not automatically run on non-members pull-requests for security
reasons. The reviewer will have to comment with
/AzurePipelines run
totrigger the build.
This change is