Skip to content

Update CONTRIBUTING.md: add Backwards Compatibility guide and update questions channels #1304

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.

Sign up for GitHub

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

Merged
HCastano merged 7 commits into from
Jul 14, 2022
Merged

Update CONTRIBUTING.md: add Backwards Compatibility guide and update questions channels #1304

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
ffd5b96
Add Backwards Compatibility guide to CONTRIBUTING.md
agryaznov Jun 27, 2022
553285a
important addition
agryaznov Jun 27, 2022
4429495
update the suggested questions channels
agryaznov Jun 27, 2022
b3e381d
Apply suggestions from code review
agryaznov Jun 28, 2022
1c57c9b
some more (tiny) corrections
agryaznov Jun 28, 2022
728d879
Wrap "Backwards Compatibility" section to 90 characters
HCastano Jul 14, 2022
a32251c
Tighten up wording a bit
HCastano Jul 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Add Backwards Compatibility guide to CONTRIBUTING.md
  • Loading branch information
@agryaznov
agryaznov committed Jun 27, 2022
commit ffd5b960964e2e5d59da982d54e88feb3f83f86b
17 changes: 17 additions & 0 deletions CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ First of all, thank you for taking your time to contribute to ink!
* [During Development](#during-development)
* [Commits](#commits)
* [Checklist](#checklist)
* [Backwards Compatibility](#backwards-compatibility)
* [Continuous Integraton](#continuous-integration)
* [Issues and Pull Requests](#issues-and-pull-requests)
* [Issues](#issues)
Expand Down Expand Up @@ -64,6 +65,22 @@ Following these will ensure that your pull request is going to be accepted.
This might seem pedantic but we believe that in essence this is going to improve overall comment and documentation quality.
1. If possible try to sign your commits, e.g. using GPG keys. For more information about this go [here](https://help.github.com/en/articles/signing-commits).

### Backwards Compatibility

ink! and pallet_contracts are the projects under active development. As contracts API functions evolve to their newer versions, we need to introduce them in ink! in a way that still keeps current *MAJOR* ink! version compatible with elder substrate nodes built with the pallet_contracts version having not this new functionality.
Copy link
Contributor

@HCastano HCastano Jun 27, 2022

Choose a reason for hiding this comment

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

Suggested change
ink! and pallet_contracts are the projects under active development. As contracts API functions evolve to their newer versions, we need to introduce them in ink! in a way that still keeps current *MAJOR* ink! version compatible with elder substrate nodes built with the pallet_contracts version having not this new functionality.
ink! and pallet_contracts are the projects under active development. As contracts API functions (denoted by the `seal_*` prefix) evolve to their newer versions, we need to introduce them in ink! in a way that still keeps current *MAJOR* ink! version compatible with older Substrate nodes. These older nodes may have a version of the Contracts pallet which does not support new seal APIs.

Copy link
Contributor Author

@agryaznov agryaznov Jun 28, 2022

Choose a reason for hiding this comment

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

left it to be Andrew's variant for the sake of concise


In order to achieve this, please stick to the following workflow.

Imagine there is a `[seal0] function()` in pallet\_contracts API and our `ink_env::function()` uses its import under the hood.
Copy link
Contributor

@HCastano HCastano Jun 27, 2022

Choose a reason for hiding this comment

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

Suggested change
Imagine there is a `[seal0] function()` in pallet\_contracts API and our `ink_env::function()` uses its import under the hood.
Imagine there is a `[seal0] function()` in the Contract pallet API and our `ink_env::function()` imports it under the hood.

Copy link
Contributor Author

@agryaznov agryaznov Jun 28, 2022

Choose a reason for hiding this comment

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

well function does not import, it uses an imported one

Then some time later we may decide to add a new `[seal1] function()` version to the pallet. In order to introduce it into ink! and still be able to run contracts (which don't use this particular function) on elder nodes, please do the following:

1. Mark the old `ink_env::function()` (which depends on the imported `[seal0]` function) with the `#[deprecated]` attribute.
Please, specify the `since` field with the ink! version since which this function is deprecated in favor of the newest one, and is left there only for the backwards compatibility purposes. Specifing some additional helpful info in the `note` field could also be a good idea.
2. Name the new function (which depends on the `[seal1] function()`) somehow descriptive like `ink_env::function_whats_special()`.
Copy link
Contributor

@HCastano HCastano Jun 27, 2022

Choose a reason for hiding this comment

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

Mark all of these as 1, the markdown render will handle the numbering

Copy link
Contributor Author

@agryaznov agryaznov Jun 28, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

well, this is an optional feature, btw it will work so even if you mark it with a randomly assigned set of integer numbers as well. However, I think it's good to have it numbered properly in the source itself.

3. Always add the `# Compatibility` section to the docs comment of the new function.
Copy link
Contributor

@HCastano HCastano Jun 27, 2022

Choose a reason for hiding this comment

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

Suggested change
3. Always add the `# Compatibility` section to the docs comment of the new function.
3. Add a `# Compatibility` section to the docs comment of the new function.


You can have a look at the [PR#1284](https://github.com/paritytech/ink/pull/1284/files#diff-e7cc1cdb3856da1293c785de863703d5961c324aa2018decb0166ea1eb0631e8R191) for a reference of how the described way could be implemented.

## Continuous Integration

Our [continuous integration (CI)](https://github.com/paritytech/ink/blob/master/.gitlab-ci.yml) will check for the following properties of all changes.
Expand Down