[
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 a check that assigns the compatibility label if we have any
change in a Java class that contains the Order annotation. The problem is that
such a check is noisy and not accurate. For instance, it also triggers for new
comments or code style changes, and the check does not trigger when a messageId
changes in CoreMessagesProvider.
Let's generate JSON representations of messages with RU-related info: the
fields, their types and order, enum values and message IDs that reviewers need
to compare. Generate the same representations for IDTOs and message IDs. CI
will generate the representations for the PR and master and will publish the
diff of the JSON files in the GitHub Job Summary to signal to a reviewer that
they should take a second look at the changes from an RU perspective.
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 the {{{}compatibility label{}}}.
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:
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 label{}}}. 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 a check that assigns the compatibility label if we have
> any change in a Java class that contains the Order annotation. The problem is
> that such a check is noisy and not accurate. For instance, it also triggers
> for new comments or code style changes, and the check does not trigger when a
> messageId changes in CoreMessagesProvider.
>
> Let's generate JSON representations of messages with RU-related info: the
> fields, their types and order, enum values and message IDs that reviewers
> need to compare. Generate the same representations for IDTOs and message IDs.
> CI will generate the representations for the PR and master and will publish
> the diff of the JSON files in the GitHub Job Summary to signal to a reviewer
> that they should take a second look at the changes from an RU perspective.
> 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 the {{{}compatibility label{}}}.
> 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)
