BotFramework-Composer icon indicating copy to clipboard operation
BotFramework-Composer copied to clipboard

fix: [#9331] Substring function contains wrong tooltip information

Open ceciliaavila opened this issue 2 years ago • 10 comments

Description

This PR fixes the tooltip description of the Substring function to reflect the current parameters.

Note: we tested the case reported in the issue:

a length from the startPos that surpasses the length of the string will generate an out of range exception, not be taken as the max length of the string.

but the exception was not thrown so we didn't update that part of the description.

Task Item

Fixes #9331

Screenshots

These images show the before and after. image

ceciliaavila avatar Aug 16 '22 13:08 ceciliaavila

Hello @OEvgeny ,

We reach out to you because we are having issues with Prs modifying extensions in BotFramework-Composer, and we saw that you created some successful PRs updating the same packages (# 9396). Our PRs fail with the error: YN0028-The lockfile would have been modified by this install, which is explicitly forbidden. And we tried different approaches to build the project and generate the yarn.berry.lock file but we couldn't make it work. We would like to know the steps you follow to build the project after updating the code of an extension, and if there's any extra configuration for the checksums to be properly updated.

Thanks!

erquirogasw avatar Oct 14 '22 20:10 erquirogasw

@erquirogasw the yarn build:dev command after some changes are made to shared packages usually works for me. If the sums didn't get updated, you could try removing node_modules from the extension package folder.

OEvgeny avatar Oct 18 '22 20:10 OEvgeny

@OEvgeny we executed the yarn build:dev command in Composer/packages/tools/built-in-functions to update the yarn files related to the changes of this PR and multiple libraries were modified in extensions (azurePublish, azurePublishNew, etc). We also tried removing the node_modules folder but more libraries were updated, compared to the ones we modified. We updated the PR with the mentioned changes in case you notice any steps that we aren't executing correctly.

erquirogasw avatar Oct 19 '22 20:10 erquirogasw

@erquirogasw please ensure you pulled the latest version from main before updating the sums.

OEvgeny avatar Oct 19 '22 22:10 OEvgeny

@OEvgeny, we cloned the repository again and after running yarn install and yarn build:dev in the Composer/ folder, we noticed that several yarn-berry.lock files were modified in extensions/. We didn't introduce any changes, just built the project on main branch. We'll try building the project directly from the GitHub action to discard an issue on our machine.

image (152)

erquirogasw avatar Oct 20 '22 19:10 erquirogasw

This might be platform-related issue. Yarn has an issue with hash persistence across platforms unfortunately.

OEvgeny avatar Oct 20 '22 21:10 OEvgeny

Coverage Status

coverage: 54.388%. remained the same when pulling 551b7ece568cb0a94d26c6367a2cf6b682279aa9 on southworks/update/substring-tooltip into e36db33876b97ba4e66660567616743f59b2b1bb on main.

coveralls avatar Oct 27 '22 15:10 coveralls

Hi @BruceHaley, we have the E2E tests failing in this PR and in a few others. Also, there's a component governance error, but we don't have access to the pipeline to understand the problem and fix it. Could you help us with this? Thanks!

ceciliaavila avatar Oct 27 '22 18:10 ceciliaavila

@ceciliaavila e2e pipeline failure seem unrelated to the PR. You can run e2e tests locally for more details

OEvgeny avatar Oct 28 '22 18:10 OEvgeny

Should be good to go after related package lock hash updates

OEvgeny avatar Apr 26 '24 01:04 OEvgeny