Use TO_TEXT_CONTRACT_ID in Show instance of ContractId #7153
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cocreature
force-pushed
the
textcontractid
branch
2 times, most recently
from
c0c1a0c to
46f41a8
Compare
fixes #7114 This PR changes the Show instance of ContractId and flips the switch on triggers and DAML Script to run in off-ledger mode. It also adds a test that for DAML Script we actually get back the correct contract id. There is a bit of a design decision here in how we want to print contract ids, so let me list the options I considered. $cid will stand for the actual cid and all options are wrapped in markdown inline code. 1. `"$cid"`. Indistinguishable from string. Suggests that there might be an IsString instance for ContractId. 2. `<$cid>`. Matches the dummy `<contract-id>` but it’s not a dummy so I don’t think matching that is benefitial. 3. `$cid`. Easy to spot (contract ids start with # and have no spaces), clearly not a string but might look slightly weird. changelog_begin changelog_end
cocreature
force-pushed
the
textcontractid
branch
from
46f41a8 to
cc7f507
Compare
hurryabit
approved these changes
Aug 17, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes #7114
This PR changes the Show instance of ContractId and flips the switch
on triggers and DAML Script to run in off-ledger mode.
It also adds a test that for DAML Script we actually get back the
correct contract id.
There is a bit of a design decision here in how we want to print
contract ids, so let me list the options I considered. $cid will stand
for the actual cid and all options are wrapped in markdown inline
code.
"$cid". Indistinguishable from string. Suggests that there mightbe an IsString instance for ContractId.
<$cid>. Matches the dummy<contract-id>but it’s not a dummy soI don’t think matching that is benefitial.
$cid. Easy to spot (contract ids start with # and have nospaces), clearly not a string but might look slightly weird.
changelog_begin
changelog_end
Pull Request Checklist
CHANGELOG_BEGINandCHANGELOG_ENDtagsNOTE: CI is not automatically run on non-members pull-requests for security
reasons. The reviewer will have to comment with
/AzurePipelines runtotrigger the build.