Skip to content

IGNITE-23344 Fixed log message in IgniteSnapshotManager::createSnapshot#11588

Merged
timoninmaxim merged 5 commits intoapache:masterfrom
vladnovoren:ignite-23344
Oct 15, 2024
Merged

IGNITE-23344 Fixed log message in IgniteSnapshotManager::createSnapshot#11588
timoninmaxim merged 5 commits intoapache:masterfrom
vladnovoren:ignite-23344

Conversation

@vladnovoren
Copy link
Contributor

@vladnovoren vladnovoren commented Oct 14, 2024

Thank you for submitting the pull request to the Apache Ignite.

In order to streamline the review of the contribution
we ask you to ensure the following steps have been taken:

The Contribution Checklist

  • There is a single JIRA ticket related to the pull request.
  • The web-link to the pull request is attached to the JIRA ticket.
  • The JIRA ticket has the Patch Available state.
  • The pull request body describes changes that have been made.
    The description explains WHAT and WHY was made instead of HOW.
  • The pull request title is treated as the final commit message.
    The following pattern must be used: IGNITE-XXXX Change summary where XXXX - number of JIRA issue.
  • A reviewer has been mentioned through the JIRA comments
    (see the Maintainers list)
  • The pull request has been checked by the Teamcity Bot and
    the green visa attached to the JIRA ticket (see TC.Bot: Check PR)

Notes

If you need any help, please email dev@ignite.apache.org or ask anу advice on http://asf.slack.com #ignite channel.

compress,
encrypt
));
var snpOpReq = new SnapshotOperationRequest(
Copy link
Member

Choose a reason for hiding this comment

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

Do not use var until you have great reason for that. Style guide for using var is here [1]. In current situation is better to use explicit type SnapshotOperationRequest. It's name is simple and the variable is used twice.

[1] https://openjdk.org/projects/amber/guides/lvti-style-guide

* */
@Test
public void testSnapshotCreationLog() throws Exception {
if (listenLog == null)
Copy link
Member

Choose a reason for hiding this comment

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

You don't need this check. listenLog must be new for every test to avoid side effects.

if (listenLog == null)
listenLog = new ListeningTestLogger(log);

final int ENTRIES_CNT = 4;
Copy link
Member

Choose a reason for hiding this comment

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

ENTRIES_CNT -> entriesCnt. Use upper case for class-level constants only [1]

[1] https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines#CodingGuidelines-Naming

LogListener matchLsnr2 = LogListener.matches("incremental=false, incIdx=-1").build();
listenLog.registerListener(matchLsnr2);

LogListener noMatchLsnr = LogListener.matches("incremental=true, incIdx=-1").build();
Copy link
Member

Choose a reason for hiding this comment

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

Let's add similar check for incremental snapshot in this test.

@sonarqubecloud
Copy link

@timoninmaxim timoninmaxim merged commit 87a22b1 into apache:master Oct 15, 2024
J-Bakuli pushed a commit to J-Bakuli/ignite that referenced this pull request Oct 28, 2024
luchnikovbsk pushed a commit to luchnikovbsk/ignite that referenced this pull request Jan 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants