Add some detail to releasing steps #1190
Merged
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
nventuro
reviewed
Aug 11, 2018
| @@ -44,8 +44,11 @@ Publish the release notes on GitHub and ask our community manager to announce th | |||
|
|
|||
| ## Creating the final release | |||
|
|
|||
| Make sure to have the latest changes from `upstream` in your local release branch. | |||
|
|
|||
| ``` | |||
| git checkout release-vX.Y.Z | |||
There was a problem hiding this comment.
I'd add a git fetch upstream here, to emphasize the need to be up-to-date with upstream (in case no fetches occured since the release branch was created).
There was a problem hiding this comment.
I don't understand, isn't this exactly what git pull is doing in the next line?
There was a problem hiding this comment.
If there is no local upstream/release-vX.Y.Z, the git checkout command will fail: a previous fetch prevents that.
There was a problem hiding this comment.
Sorry, I still don't understand how this relates to my proposed steps.
There was a problem hiding this comment.
The readme says:
Make sure to have the latest changes from
upstreamin your local release branch.
However, if the branch was created from a different computer, the one the final release is being made from may have not performed a fetch since then, and will therefore not know about the upstream/release-vX.Y.Z branch, making checkout fail. A previous explicit fetch prevents that.
This is, granted, a somewhat contrived scenario, and I'd expect the releaser to know enough of git to not need this help, but I'd still add it for completeness sake.
There was a problem hiding this comment.
I prefer to leave the snippets simple even if somewhat incomplete, and later automate all of this in a script. Does that sound ok?
|
What is the procedure for merging back into |
|
@nventuro It's specified in the last section of the document. I added a similar comment to that section. |
nventuro
approved these changes
Aug 24, 2018
nventuro
added a commit
that referenced
this pull request
Sep 4, 2018
* Minor test style improvements (#1219) * Changed .eq to .equal * Changed equal(bool) to .to.be.bool * Changed be.bool to equal(bool), disallowed unused expressions. * Add ERC165Query library (#1086) * Add ERC165Query library * Address PR Comments * Add tests and mocks from #1024 and refactor code slightly * Fix javascript and solidity linting errors * Split supportsInterface into three methods as discussed in #1086 * Change InterfaceId_ERC165 comment to match style in the rest of the repo * Fix max-len lint issue on ERC165Checker.sol * Conditionally ignore the asserts during solidity-coverage test * Switch to abi.encodeWithSelector and add test for account addresses * Switch to supportsInterfaces API as suggested by @frangio * Adding ERC165InterfacesSupported.sol * Fix style issues * Add test for supportsInterfaces returning false * Add ERC165Checker.sol newline * feat: fix coverage implementation * fix: solidity linting error * fix: revert to using boolean tests instead of require statements * fix: make supportsERC165Interface private again * rename SupportsInterfaceWithLookupMock to avoid name clashing * Added mint and burn tests for zero amounts. (#1230) * Changed .eq to .equal. (#1231) * ERC721 pausable token (#1154) * ERC721 pausable token * Reuse of ERC721 Basic behavior for Pausable, split view checks in paused state & style fixes * [~] paused token behavior * Add some detail to releasing steps (#1190) * add note about pulling upstream changes to release branch * add comment about upstream changes in merging section * Increase test coverage (#1237) * Fixed a SplitPayment test * Deleted unnecessary function. * Improved PostDeliveryCrowdsale tests. * Improved RefundableCrowdsale tests. * Improved MintedCrowdsale tests. * Improved IncreasingPriceCrowdsale tests. * Fixed a CappedCrowdsale test. * Improved TimedCrowdsale tests. * Improved descriptions of added tests. * ci: trigger docs update on tag (#1186) * MintableToken now uses Roles. * Fixed FinalizableCrowdsale test. * Roles can now be transfered. * Fixed tests related to MintableToken. * Removed Roles.check. * Renamed transferMintPermission. * Moved MinterRole * Fixed RBAC. * Adressed review comments. * Addressed review comments * Fixed linter errors. * Added Events tests of Pausable contract (#1207) * Fixed roles tests. * Rename events to past-tense (#1181) * fix: refactor sign.js and related tests (#1243) * fix: refactor sign.js and related tests * fix: remove unused dep * fix: update package.json correctly * Added "_" sufix to internal variables (#1171) * Added PublicRole test. * Fixed crowdsale tests. * Rename ERC interfaces to I prefix (#1252) * rename ERC20 to IERC20 * move ERC20.sol to IERC20.sol * rename StandardToken to ERC20 * rename StandardTokenMock to ERC20Mock * move StandardToken.sol to ERC20.sol, likewise test and mock files * rename MintableToken to ERC20Mintable * move MintableToken.sol to ERC20Mintable.sol, likewise test and mock files * rename BurnableToken to ERC20Burnable * move BurnableToken.sol to ERC20Burnable.sol, likewise for related files * rename CappedToken to ERC20Capped * move CappedToken.sol to ERC20Capped.sol, likewise for related files * rename PausableToken to ERC20Pausable * move PausableToken.sol to ERC20Pausable.sol, likewise for related files * rename DetailedERC20 to ERC20Detailed * move DetailedERC20.sol to ERC20Detailed.sol, likewise for related files * rename ERC721 to IERC721, and likewise for other related interfaces * move ERC721.sol to IERC721.sol, likewise for other 721 interfaces * rename ERC721Token to ERC721 * move ERC721Token.sol to ERC721.sol, likewise for related files * rename ERC721BasicToken to ERC721Basic * move ERC721BasicToken.sol to ERC721Basic.sol, likewise for related files * rename ERC721PausableToken to ERC721Pausable * move ERC721PausableToken.sol to ERC721Pausable.sol * rename ERC165 to IERC165 * move ERC165.sol to IERC165.sol * amend comment that ERC20 is based on FirstBlood * fix comments mentioning IERC721Receiver * added explicit visibility (#1261) * Remove underscores from event parameters. (#1258) * Remove underscores from event parameters. Fixes #1175 * Add comment about ERC * Move contracts to subdirectories (#1253) * Move contracts to subdirectories Fixes #1177. This Change also removes the LimitBalance contract. * fix import * move MerkleProof to cryptography * Fix import * Remove HasNoEther, HasNoTokens, HasNoContracts, and NoOwner (#1254) * remove HasNoEther, HasNoTokens, HasNoContracts, and NoOwner * remove unused ERC223TokenMock * remove Contactable * remove TokenDestructible * remove DeprecatedERC721 * inline Destructible#destroy in Bounty * remove Destructible * Functions in interfaces changed to "external" (#1263) * Add a leading underscore to internal and private functions. (#1257) * Add a leading underscore to internal and private functions. Fixes #1176 * Remove super * update the ERC721 changes * add missing underscore after merge * Fix mock * Improve encapsulation on SignatureBouncer, Whitelist and RBAC example (#1265) * Improve encapsulation on Whitelist * remove only * update whitelisted crowdsale test * Improve encapsulation on SignatureBouncer * fix missing test * Improve encapsulation on RBAC example * Improve encapsulation on RBAC example * Remove extra visibility * Improve encapsulation on ERC20 Mintable * Improve encapsulation on Superuser * fix lint * add missing constant * Addressed review comments. * Fixed build error.
nventuro
added a commit
that referenced
this pull request
Sep 7, 2018
* Role tests (#1228) * Moved RBAC tests to access. * Added Roles.addMany and tests. * Fixed linter error. * Now using uint256 indexes. * Removed RBAC tokens (#1229) * Deleted RBACCappedTokenMock. * Removed RBACMintableToken. * Removed RBACMintableToken from the MintedCrowdsale tests. * Roles can now be transfered. (#1235) * Roles can now be transfered. * Now explicitly checking support for the null address. * Now rejecting transfer to a role-haver. * Added renounce, roles can no longer be transfered to 0. * Fixed linter errors. * Fixed a Roles test. * True Ownership (#1247) * Added barebones Secondary. * Added transferPrimary * Escrow is now Secondary instead of Ownable. * Now reverting on transfers to 0. * The Secondary's primary is now private. * MintableToken using Roles (#1236) * Minor test style improvements (#1219) * Changed .eq to .equal * Changed equal(bool) to .to.be.bool * Changed be.bool to equal(bool), disallowed unused expressions. * Add ERC165Query library (#1086) * Add ERC165Query library * Address PR Comments * Add tests and mocks from #1024 and refactor code slightly * Fix javascript and solidity linting errors * Split supportsInterface into three methods as discussed in #1086 * Change InterfaceId_ERC165 comment to match style in the rest of the repo * Fix max-len lint issue on ERC165Checker.sol * Conditionally ignore the asserts during solidity-coverage test * Switch to abi.encodeWithSelector and add test for account addresses * Switch to supportsInterfaces API as suggested by @frangio * Adding ERC165InterfacesSupported.sol * Fix style issues * Add test for supportsInterfaces returning false * Add ERC165Checker.sol newline * feat: fix coverage implementation * fix: solidity linting error * fix: revert to using boolean tests instead of require statements * fix: make supportsERC165Interface private again * rename SupportsInterfaceWithLookupMock to avoid name clashing * Added mint and burn tests for zero amounts. (#1230) * Changed .eq to .equal. (#1231) * ERC721 pausable token (#1154) * ERC721 pausable token * Reuse of ERC721 Basic behavior for Pausable, split view checks in paused state & style fixes * [~] paused token behavior * Add some detail to releasing steps (#1190) * add note about pulling upstream changes to release branch * add comment about upstream changes in merging section * Increase test coverage (#1237) * Fixed a SplitPayment test * Deleted unnecessary function. * Improved PostDeliveryCrowdsale tests. * Improved RefundableCrowdsale tests. * Improved MintedCrowdsale tests. * Improved IncreasingPriceCrowdsale tests. * Fixed a CappedCrowdsale test. * Improved TimedCrowdsale tests. * Improved descriptions of added tests. * ci: trigger docs update on tag (#1186) * MintableToken now uses Roles. * Fixed FinalizableCrowdsale test. * Roles can now be transfered. * Fixed tests related to MintableToken. * Removed Roles.check. * Renamed transferMintPermission. * Moved MinterRole * Fixed RBAC. * Adressed review comments. * Addressed review comments * Fixed linter errors. * Added Events tests of Pausable contract (#1207) * Fixed roles tests. * Rename events to past-tense (#1181) * fix: refactor sign.js and related tests (#1243) * fix: refactor sign.js and related tests * fix: remove unused dep * fix: update package.json correctly * Added "_" sufix to internal variables (#1171) * Added PublicRole test. * Fixed crowdsale tests. * Rename ERC interfaces to I prefix (#1252) * rename ERC20 to IERC20 * move ERC20.sol to IERC20.sol * rename StandardToken to ERC20 * rename StandardTokenMock to ERC20Mock * move StandardToken.sol to ERC20.sol, likewise test and mock files * rename MintableToken to ERC20Mintable * move MintableToken.sol to ERC20Mintable.sol, likewise test and mock files * rename BurnableToken to ERC20Burnable * move BurnableToken.sol to ERC20Burnable.sol, likewise for related files * rename CappedToken to ERC20Capped * move CappedToken.sol to ERC20Capped.sol, likewise for related files * rename PausableToken to ERC20Pausable * move PausableToken.sol to ERC20Pausable.sol, likewise for related files * rename DetailedERC20 to ERC20Detailed * move DetailedERC20.sol to ERC20Detailed.sol, likewise for related files * rename ERC721 to IERC721, and likewise for other related interfaces * move ERC721.sol to IERC721.sol, likewise for other 721 interfaces * rename ERC721Token to ERC721 * move ERC721Token.sol to ERC721.sol, likewise for related files * rename ERC721BasicToken to ERC721Basic * move ERC721BasicToken.sol to ERC721Basic.sol, likewise for related files * rename ERC721PausableToken to ERC721Pausable * move ERC721PausableToken.sol to ERC721Pausable.sol * rename ERC165 to IERC165 * move ERC165.sol to IERC165.sol * amend comment that ERC20 is based on FirstBlood * fix comments mentioning IERC721Receiver * added explicit visibility (#1261) * Remove underscores from event parameters. (#1258) * Remove underscores from event parameters. Fixes #1175 * Add comment about ERC * Move contracts to subdirectories (#1253) * Move contracts to subdirectories Fixes #1177. This Change also removes the LimitBalance contract. * fix import * move MerkleProof to cryptography * Fix import * Remove HasNoEther, HasNoTokens, HasNoContracts, and NoOwner (#1254) * remove HasNoEther, HasNoTokens, HasNoContracts, and NoOwner * remove unused ERC223TokenMock * remove Contactable * remove TokenDestructible * remove DeprecatedERC721 * inline Destructible#destroy in Bounty * remove Destructible * Functions in interfaces changed to "external" (#1263) * Add a leading underscore to internal and private functions. (#1257) * Add a leading underscore to internal and private functions. Fixes #1176 * Remove super * update the ERC721 changes * add missing underscore after merge * Fix mock * Improve encapsulation on SignatureBouncer, Whitelist and RBAC example (#1265) * Improve encapsulation on Whitelist * remove only * update whitelisted crowdsale test * Improve encapsulation on SignatureBouncer * fix missing test * Improve encapsulation on RBAC example * Improve encapsulation on RBAC example * Remove extra visibility * Improve encapsulation on ERC20 Mintable * Improve encapsulation on Superuser * fix lint * add missing constant * Addressed review comments. * Fixed build error. * Improved Roles API. (#1280) * Improved Roles API. * fix linter error * Added PauserRole. (#1283) * Remove Claimable, DelayedClaimable, Heritable (#1274) * remove Claimable, DelayedClaimable, Heritable * remove SimpleSavingsWallet example which used Heritable (cherry picked from commit 0dc7117) * Role behavior tests (#1285) * Added role tests. * Added PauserRole tests to contracts that have that role. * Added MinterRole tests to contracts that have that role. * Fixed linter errors. * Migrate Ownable to Roles (#1287) * Added CapperRole. * RefundEscrow is now Secondary. * FinalizableCrowdsale is no longer Ownable. * Removed Whitelist and WhitelistedCrowdsale, redesign needed. * Fixed linter errors, disabled lbrace due to it being buggy. * Remove RBAC, SignatureBouncer refactor (#1289) * Added CapperRole. * RefundEscrow is now Secondary. * FinalizableCrowdsale is no longer Ownable. * Removed Whitelist and WhitelistedCrowdsale, redesign needed. * Fixed linter errors, disabled lbrace due to it being buggy. * Moved SignatureBouncer tests. * Deleted RBAC and Superuser. * Deleted rbac directory. * Updated readme. * SignatureBouncer now uses SignerRole, renamed bouncer to signer. * feat: implement ERC721Mintable and ERC721Burnable (#1276) * feat: implement ERC721Mintable and ERC721Burnable * fix: linting errors * fix: remove unused mintable mock for ERC721BasicMock * fix: add finishMinting tests * fix: catch MintFinished typo * inline ERC721Full behavior * undo pretty formatting * fix lint errors * rename canMint to onlyBeforeMintingFinished for consistency with ERC20Mintable * Fix the merge with the privatization branch * remove duplicate CapperRole test
frangio
pushed a commit
that referenced
this pull request
Sep 7, 2018
…meters (#1282) * Role tests (#1228) * Moved RBAC tests to access. * Added Roles.addMany and tests. * Fixed linter error. * Now using uint256 indexes. * Removed RBAC tokens (#1229) * Deleted RBACCappedTokenMock. * Removed RBACMintableToken. * Removed RBACMintableToken from the MintedCrowdsale tests. * Roles can now be transfered. (#1235) * Roles can now be transfered. * Now explicitly checking support for the null address. * Now rejecting transfer to a role-haver. * Added renounce, roles can no longer be transfered to 0. * Fixed linter errors. * Fixed a Roles test. * True Ownership (#1247) * Added barebones Secondary. * Added transferPrimary * Escrow is now Secondary instead of Ownable. * Now reverting on transfers to 0. * The Secondary's primary is now private. * Improve encapsulation on ERC165 * Improve encapsulation on ERC20 * Improve encapsulation on ERC721 * Add tests, use standard getters * fix tests * Fix lint * MintableToken using Roles (#1236) * Minor test style improvements (#1219) * Changed .eq to .equal * Changed equal(bool) to .to.be.bool * Changed be.bool to equal(bool), disallowed unused expressions. * Add ERC165Query library (#1086) * Add ERC165Query library * Address PR Comments * Add tests and mocks from #1024 and refactor code slightly * Fix javascript and solidity linting errors * Split supportsInterface into three methods as discussed in #1086 * Change InterfaceId_ERC165 comment to match style in the rest of the repo * Fix max-len lint issue on ERC165Checker.sol * Conditionally ignore the asserts during solidity-coverage test * Switch to abi.encodeWithSelector and add test for account addresses * Switch to supportsInterfaces API as suggested by @frangio * Adding ERC165InterfacesSupported.sol * Fix style issues * Add test for supportsInterfaces returning false * Add ERC165Checker.sol newline * feat: fix coverage implementation * fix: solidity linting error * fix: revert to using boolean tests instead of require statements * fix: make supportsERC165Interface private again * rename SupportsInterfaceWithLookupMock to avoid name clashing * Added mint and burn tests for zero amounts. (#1230) * Changed .eq to .equal. (#1231) * ERC721 pausable token (#1154) * ERC721 pausable token * Reuse of ERC721 Basic behavior for Pausable, split view checks in paused state & style fixes * [~] paused token behavior * Add some detail to releasing steps (#1190) * add note about pulling upstream changes to release branch * add comment about upstream changes in merging section * Increase test coverage (#1237) * Fixed a SplitPayment test * Deleted unnecessary function. * Improved PostDeliveryCrowdsale tests. * Improved RefundableCrowdsale tests. * Improved MintedCrowdsale tests. * Improved IncreasingPriceCrowdsale tests. * Fixed a CappedCrowdsale test. * Improved TimedCrowdsale tests. * Improved descriptions of added tests. * ci: trigger docs update on tag (#1186) * MintableToken now uses Roles. * Fixed FinalizableCrowdsale test. * Roles can now be transfered. * Fixed tests related to MintableToken. * Removed Roles.check. * Renamed transferMintPermission. * Moved MinterRole * Fixed RBAC. * Adressed review comments. * Addressed review comments * Fixed linter errors. * Added Events tests of Pausable contract (#1207) * Fixed roles tests. * Rename events to past-tense (#1181) * fix: refactor sign.js and related tests (#1243) * fix: refactor sign.js and related tests * fix: remove unused dep * fix: update package.json correctly * Added "_" sufix to internal variables (#1171) * Added PublicRole test. * Fixed crowdsale tests. * Rename ERC interfaces to I prefix (#1252) * rename ERC20 to IERC20 * move ERC20.sol to IERC20.sol * rename StandardToken to ERC20 * rename StandardTokenMock to ERC20Mock * move StandardToken.sol to ERC20.sol, likewise test and mock files * rename MintableToken to ERC20Mintable * move MintableToken.sol to ERC20Mintable.sol, likewise test and mock files * rename BurnableToken to ERC20Burnable * move BurnableToken.sol to ERC20Burnable.sol, likewise for related files * rename CappedToken to ERC20Capped * move CappedToken.sol to ERC20Capped.sol, likewise for related files * rename PausableToken to ERC20Pausable * move PausableToken.sol to ERC20Pausable.sol, likewise for related files * rename DetailedERC20 to ERC20Detailed * move DetailedERC20.sol to ERC20Detailed.sol, likewise for related files * rename ERC721 to IERC721, and likewise for other related interfaces * move ERC721.sol to IERC721.sol, likewise for other 721 interfaces * rename ERC721Token to ERC721 * move ERC721Token.sol to ERC721.sol, likewise for related files * rename ERC721BasicToken to ERC721Basic * move ERC721BasicToken.sol to ERC721Basic.sol, likewise for related files * rename ERC721PausableToken to ERC721Pausable * move ERC721PausableToken.sol to ERC721Pausable.sol * rename ERC165 to IERC165 * move ERC165.sol to IERC165.sol * amend comment that ERC20 is based on FirstBlood * fix comments mentioning IERC721Receiver * added explicit visibility (#1261) * Remove underscores from event parameters. (#1258) * Remove underscores from event parameters. Fixes #1175 * Add comment about ERC * Move contracts to subdirectories (#1253) * Move contracts to subdirectories Fixes #1177. This Change also removes the LimitBalance contract. * fix import * move MerkleProof to cryptography * Fix import * Remove HasNoEther, HasNoTokens, HasNoContracts, and NoOwner (#1254) * remove HasNoEther, HasNoTokens, HasNoContracts, and NoOwner * remove unused ERC223TokenMock * remove Contactable * remove TokenDestructible * remove DeprecatedERC721 * inline Destructible#destroy in Bounty * remove Destructible * Functions in interfaces changed to "external" (#1263) * Add a leading underscore to internal and private functions. (#1257) * Add a leading underscore to internal and private functions. Fixes #1176 * Remove super * update the ERC721 changes * add missing underscore after merge * Fix mock * Improve encapsulation on SignatureBouncer, Whitelist and RBAC example (#1265) * Improve encapsulation on Whitelist * remove only * update whitelisted crowdsale test * Improve encapsulation on SignatureBouncer * fix missing test * Improve encapsulation on RBAC example * Improve encapsulation on RBAC example * Remove extra visibility * Improve encapsulation on ERC20 Mintable * Improve encapsulation on Superuser * fix lint * add missing constant * Addressed review comments. * Fixed build error. * move interface ids to implementation contracts * Do not prefix getters * Improve encapsulation on Crowdsales * add missing tests * remove only * Improve encapsulation on Pausable * add the underscore * Improve encapsulation on ownership * fix rebase * fix ownership * Improve encapsulation on payments * Add missing tests * add missing test * Do not prefix getters * Do not prefix getters * Fix tests. * Update modifiers to call public view functions. Fixes #1179. * Improve encapsulation on BreakInvariantBounty * Make researchers private * Improve encapsulation on Crowdsales * add missing tests * remove only * Improve encapsulation on Pausable * add the underscore * Improve encapsulation on ownership * fix rebase * fix ownership * Improve encapsulation on payments * Add missing tests * add missing test * Do not prefix getters * Do not prefix getters * Do not prefix getters * Fix tests. * tmp * remove isMinter * fix is owner call * fix isOpen * Fix merge * tmp * Improve encapsulation on TimedCrowdsale * Add missing parentheses * Use prefix underscore for state variables and no underscore for parameters * Improved Roles API. (#1280) * Improved Roles API. * fix linter error * Added PauserRole. (#1283) * remove duplicate function definition * Remove Claimable, DelayedClaimable, Heritable (#1274) * remove Claimable, DelayedClaimable, Heritable * remove SimpleSavingsWallet example which used Heritable (cherry picked from commit 0dc7117) * Role behavior tests (#1285) * Added role tests. * Added PauserRole tests to contracts that have that role. * Added MinterRole tests to contracts that have that role. * Fixed linter errors. * Migrate Ownable to Roles (#1287) * Added CapperRole. * RefundEscrow is now Secondary. * FinalizableCrowdsale is no longer Ownable. * Removed Whitelist and WhitelistedCrowdsale, redesign needed. * Fixed linter errors, disabled lbrace due to it being buggy. * Remove RBAC, SignatureBouncer refactor (#1289) * Added CapperRole. * RefundEscrow is now Secondary. * FinalizableCrowdsale is no longer Ownable. * Removed Whitelist and WhitelistedCrowdsale, redesign needed. * Fixed linter errors, disabled lbrace due to it being buggy. * Moved SignatureBouncer tests. * Deleted RBAC and Superuser. * Deleted rbac directory. * Updated readme. * SignatureBouncer now uses SignerRole, renamed bouncer to signer. * feat: implement ERC721Mintable and ERC721Burnable (#1276) * feat: implement ERC721Mintable and ERC721Burnable * fix: linting errors * fix: remove unused mintable mock for ERC721BasicMock * fix: add finishMinting tests * fix: catch MintFinished typo * inline ERC721Full behavior * undo pretty formatting * fix lint errors * rename canMint to onlyBeforeMintingFinished for consistency with ERC20Mintable * Fix the merge with the privatization branch * fix lint * Remove underscore * Delete CapperRole.test.js * fix increaseApproval
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.
No description provided.