[
https://issues.apache.org/jira/browse/IGNITE-25489?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Aleksandr Chesnokov updated IGNITE-25489:
-----------------------------------------
Description:
To maintain RU compatibility we have to review new PRs carefully and control
wire changes.
Reviewers should see wire format changes in PRs to {{master}} before merge. For
now we have only check that assigns label compatibility if we have any change
in java class that contains Order annotation. The problem is that such check is
noisy and not accurate. For instance, it also triggers for new comments or
codestyle changes and the check does not trigger on changing messageId in
CoreMessagesProvider
Let's generate JSON representation of messages with RU-related info: the
fields, their types and order, enum values and message IDs that reviewers need
to compare. Same representations for idto and message ids. CI will generate the
representations for PR and master and will publish the diff of json files in
GitHub Job Summary to signal a reviewer to look the changes twice from point of
RU.
h3. What is in place
* IGNITE-29103 generates descriptions of messages and enums. [PR
#13675|https://github.com/apache/ignite/pull/13675]
* IGNITE-29143 tracks the JSON in Git and checks that it is up to date. [PR
#13693|https://github.com/apache/ignite/pull/13693]
* IGNITE-29146 adds IDTO descriptions. [PR
#13730|https://github.com/apache/ignite/pull/13730]
* IGNITE-29144 records the mapping from message IDs to classes in
{{{}ids.json{}}}. [PR #13763|https://github.com/apache/ignite/pull/13763]
h3. Deliver in this order
# *IGNITE-29261 generates and compares JSON in CI.* Remove the generated files
from Git. CI compares the target branch snapshot with GitHub's test merge of
the PR. It saves the full diff as an artifact. The comparison works on target
branches that have the required generators. [PR
#13802|https://github.com/apache/ignite/pull/13802]
# *IGNITE-29145 reports the result on the PR.* After IGNITE-29261 merges, a
trusted {{workflow_run}} workflow reads the comparison and completeness
results. It updates the bot comment and the label for the current PR head.
h3. Done when
On a PR to {{{}master{}}}, including a fork PR, CI shows the diff in Job
Summary. The {{check-wire}} job warns when the format changes. A build or
comparison error fails the check. When the comparison and JSON completeness
check pass and there is a diff, the bot adds {{{}compatibility{}}}. It posts or
updates one comment linking to Job Summary.
Reviewers use the diff to decide what needs a closer look. They still need
compatibility tests and manual review, especially for changes to hand-written
serialization.
h3. Later work
* IGNITE-29289 tests whether a hint can reach committers before CI approval on
a fork PR.
* IGNITE-29230 explores rules for explaining compatibility changes from
generated descriptions.
* IGNITE-29161 evaluates whether a code hash can help reviewers spot changes
to serialization code.
[Ignite dev-list
discussion|https://lists.apache.org/thread.html/hq3d8opxscww09pobvqfvwgc1d0v9cls]
was:
Reviewers should see wire format changes in PRs to {{master}} before merge.
Contributors should only need to commit their source changes. A Java-text check
can flag formatting and miss changes to message IDs. JSON describes the fields,
their types and order, enum values and message IDs that reviewers need to
compare.
h3. What is in place
* IGNITE-29103 generates descriptions of messages and enums. [PR
#13675|https://github.com/apache/ignite/pull/13675]
* IGNITE-29143 tracks the JSON in Git and checks that it is up to date. [PR
#13693|https://github.com/apache/ignite/pull/13693]
* IGNITE-29146 adds IDTO descriptions. [PR
#13730|https://github.com/apache/ignite/pull/13730]
* IGNITE-29144 records the mapping from message IDs to classes in {{ids.json}}.
[PR #13763|https://github.com/apache/ignite/pull/13763]
h3. Deliver in this order
# *IGNITE-29261 generates and compares JSON in CI.* Remove the generated files
from Git. CI compares the target branch snapshot with GitHub's test merge of
the PR. It saves the full diff as an artifact. The comparison works on target
branches that have the required generators. [PR
#13802|https://github.com/apache/ignite/pull/13802]
# *IGNITE-29145 reports the result on the PR.* After IGNITE-29261 merges, a
trusted {{workflow_run}} workflow reads the comparison and completeness
results. It updates the bot comment and the label for the current PR head.
h3. Done when
On a PR to {{master}}, including a fork PR, CI shows the diff in Job Summary.
The {{check-wire}} job warns when the format changes. A build or comparison
error fails the check. When the comparison and JSON completeness check pass and
there is a diff, the bot adds {{compatibility}}. It posts or updates one
comment linking to Job Summary.
Reviewers use the diff to decide what needs a closer look. They still need
compatibility tests and manual review, especially for changes to hand-written
serialization.
h3. Later work
* IGNITE-29289 tests whether a hint can reach committers before CI approval on
a fork PR.
* IGNITE-29230 explores rules for explaining compatibility changes from
generated descriptions.
* IGNITE-29161 evaluates whether a code hash can help reviewers spot changes to
serialization code.
[Ignite dev-list
discussion|https://lists.apache.org/thread.html/hq3d8opxscww09pobvqfvwgc1d0v9cls]
> [Umbrella] Track wire format changes for rolling upgrade compatibility
> ----------------------------------------------------------------------
>
> Key: IGNITE-25489
> URL: https://issues.apache.org/jira/browse/IGNITE-25489
> Project: Ignite
> Issue Type: Improvement
> Reporter: Maksim Timonin
> Assignee: Aleksandr Chesnokov
> Priority: Major
> Labels: IEP-132, ise
> Time Spent: 1h 20m
> Remaining Estimate: 0h
>
> To maintain RU compatibility we have to review new PRs carefully and control
> wire changes.
> Reviewers should see wire format changes in PRs to {{master}} before merge.
> For now we have only check that assigns label compatibility if we have any
> change in java class that contains Order annotation. The problem is that such
> check is noisy and not accurate. For instance, it also triggers for new
> comments or codestyle changes and the check does not trigger on changing
> messageId in CoreMessagesProvider
>
> Let's generate JSON representation of messages with RU-related info: the
> fields, their types and order, enum values and message IDs that reviewers
> need to compare. Same representations for idto and message ids. CI will
> generate the representations for PR and master and will publish the diff of
> json files in GitHub Job Summary to signal a reviewer to look the changes
> twice from point of RU.
> h3. What is in place
> * IGNITE-29103 generates descriptions of messages and enums. [PR
> #13675|https://github.com/apache/ignite/pull/13675]
> * IGNITE-29143 tracks the JSON in Git and checks that it is up to date. [PR
> #13693|https://github.com/apache/ignite/pull/13693]
> * IGNITE-29146 adds IDTO descriptions. [PR
> #13730|https://github.com/apache/ignite/pull/13730]
> * IGNITE-29144 records the mapping from message IDs to classes in
> {{{}ids.json{}}}. [PR #13763|https://github.com/apache/ignite/pull/13763]
> h3. Deliver in this order
> # *IGNITE-29261 generates and compares JSON in CI.* Remove the generated
> files from Git. CI compares the target branch snapshot with GitHub's test
> merge of the PR. It saves the full diff as an artifact. The comparison works
> on target branches that have the required generators. [PR
> #13802|https://github.com/apache/ignite/pull/13802]
> # *IGNITE-29145 reports the result on the PR.* After IGNITE-29261 merges, a
> trusted {{workflow_run}} workflow reads the comparison and completeness
> results. It updates the bot comment and the label for the current PR head.
> h3. Done when
> On a PR to {{{}master{}}}, including a fork PR, CI shows the diff in Job
> Summary. The {{check-wire}} job warns when the format changes. A build or
> comparison error fails the check. When the comparison and JSON completeness
> check pass and there is a diff, the bot adds {{{}compatibility{}}}. It posts
> or updates one comment linking to Job Summary.
> Reviewers use the diff to decide what needs a closer look. They still need
> compatibility tests and manual review, especially for changes to hand-written
> serialization.
> h3. Later work
> * IGNITE-29289 tests whether a hint can reach committers before CI approval
> on a fork PR.
> * IGNITE-29230 explores rules for explaining compatibility changes from
> generated descriptions.
> * IGNITE-29161 evaluates whether a code hash can help reviewers spot changes
> to serialization code.
> [Ignite dev-list
> discussion|https://lists.apache.org/thread.html/hq3d8opxscww09pobvqfvwgc1d0v9cls]
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
