Skip to content

Conversation

@tombogle
Copy link
Contributor

@tombogle tombogle commented May 27, 2025

Also:

  • Deprecated AbandondedSuites in favor of correctly spelled AbandonedSuites
  • Misc minor code refactoring and cleanup.
  • Fixed typos in comments.

This change is Reviewable

…otations

Deprecated AbandondedSuites in favor of correctly spelled AbandonedSuites
Misc minor code refactoring and cleanup.
Fixed typos in comments.
@tombogle tombogle requested a review from ermshiperete May 27, 2025 17:59
@tombogle tombogle self-assigned this May 27, 2025
@github-actions
Copy link

github-actions bot commented May 27, 2025

Test Results

  4 files  ±0   46 suites  ±0   1m 10s ⏱️ +28s
195 tests ±0  195 ✅ ±0  0 💤 ±0  0 ❌ ±0 
390 runs  ±0  388 ✅ ±0  2 💤 ±0  0 ❌ ±0 

Results for commit a20c5e7. ± Comparison against base commit da2bbcc.

♻️ This comment has been updated with latest results.

Copy link

@imnasnainaec imnasnainaec left a comment

Choose a reason for hiding this comment

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

Reviewed 15 of 26 files at r1, all commit messages.
Reviewable status: 15 of 26 files reviewed, all discussions resolved (waiting on @ermshiperete)


SIL.BuildTasks/FileUpdate.cs line 1 at r1 (raw file):

// Copyright (c) 2023 SIL Global

2023-2025


SIL.BuildTasks.AWS/S3/S3BuildPublisher.cs line 84 at r1 (raw file):

		private bool ProcessFiles()
		{
			Log.LogMessage(MessageImportance.Normal, "Publishing SourceFiles={0} to {1}", Join(SourceFiles), DestinationBucket);

Is changing a log message possibly a breaking change?


SIL.BuildTasks/StampAssemblies/StampAssemblies.cs line 1 at r1 (raw file):

// Copyright (c) 2025 SIL Global

2018-2025


SIL.BuildTasks/UnitTestTasks/TestTask.cs line 1 at r1 (raw file):

// Copyright (c) 20218-2025 SIL Global

2018-2025

Copy link
Contributor Author

@tombogle tombogle left a comment

Choose a reason for hiding this comment

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

Reviewable status: 15 of 26 files reviewed, all discussions resolved (waiting on @ermshiperete)


SIL.BuildTasks.AWS/S3/S3BuildPublisher.cs line 84 at r1 (raw file):

Previously, imnasnainaec (D. Ror.) wrote…

Is changing a log message possibly a breaking change?

I did consider that possibility. I wouldn't think so, but I'm not sure how to verify for sure.

Copy link
Member

@ermshiperete ermshiperete left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@ermshiperete ermshiperete left a comment

Choose a reason for hiding this comment

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

Reviewed 22 of 26 files at r1, 4 of 4 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @tombogle)

Copy link

@imnasnainaec imnasnainaec left a comment

Choose a reason for hiding this comment

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

Reviewed all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @tombogle)

@tombogle tombogle merged commit cd70c12 into master May 28, 2025
6 checks passed
@tombogle tombogle deleted the use-jetbrains-annotations branch May 28, 2025 13:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants