Skip to content

task: Add FilesCheck option to breaking check#227

Open
hound672 wants to merge 3 commits into
mainfrom
task/breaking-check-file
Open

task: Add FilesCheck option to breaking check#227
hound672 wants to merge 3 commits into
mainfrom
task/breaking-check-file

Conversation

@hound672
Copy link
Copy Markdown
Contributor

No description provided.

hound672 added 3 commits May 14, 2026 23:28
* config breaking check

* config breaking check
@hound672 hound672 changed the title task: A task: Add FilesCheck option to breaking check May 17, 2026
@Yakwilik Yakwilik requested a review from Copilot May 17, 2026 23:22
@Yakwilik
Copy link
Copy Markdown
Member

Yakwilik commented May 17, 2026

Стоит обновить валидатор и генерацию схемы конфига.
И доку

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds an opt-in “FilesCheck” mode to the breaking-change checker so that moving protobuf entities between .proto files can be treated as a breaking change, and wires this option through config and core initialization.

Changes:

  • Introduces filesCheck support in BreakingChecker and emits new “moved” issues for services/messages/oneofs/enums when enabled.
  • Extends breaking-check configuration to accept a breaking.use list (e.g. FILES_CHECK) and maps it into core.BreakingCheckConfig.FilesCheck.
  • Updates the example config to demonstrate enabling FILES_CHECK.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
internal/core/breaking_checker.go Adds optional file-move detection and new “moved” issue constructors.
internal/core/breaking_check.go Adds config constant and threads FilesCheck into BreakingChecker.
internal/config/breaking_check.go Extends YAML/JSON config model with use.
internal/api/temporaly_helper.go Maps breaking.use into core.BreakingCheckConfig.FilesCheck.
example.easyp.yaml Documents enabling FILES_CHECK in config.
Comments suppressed due to low confidence (2)

internal/core/breaking_checker.go:264

  • hasFilesCheckError compares ProtoFilePath values coming from different walkers: the on-disk walker yields paths relative to breakingCheckRoot, while the git-tree walker yields repo-relative paths (e.g. with a subdir prefix when breakingCheckRoot is not the repo root). With filesCheck enabled this will produce false “moved” issues for unchanged files. Normalize both paths to the same base (preferably repo-relative) before comparing, or adjust the git walker to report paths relative to the requested root/path so both sides match.
func (b *BreakingChecker) hasFilesCheckError(path1, path2 string) bool {
	if b.filesCheck && path1 != path2 {
		return true
	}

	return false
}

internal/core/breaking_checker.go:447

  • These new “moved” issue messages include a trailing newline (\n), but the text formatter already appends \n when printing each issue. This will introduce extra blank lines in text output; consider removing the trailing newline from the message strings for consistency with most existing breaking-check messages.
	message := fmt.Sprintf(
		"Previously present Message \"%s\" was moved from \"%s\" file to \"%s\" file\n",
		againstMessage.MessagePath,
		againstMessage.ProtoFilePath,
		currentMessage.ProtoFilePath,
	)
	return buildBreakingCheckIssue(againstMessage.ProtoFilePath, message, againstMessage.Meta.Pos)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +7 to 9
AgainstGitRef string `json:"against_git_ref,omitempty" yaml:"against_git_ref,omitempty"`
Use []string `json:"use,omitempty" yaml:"use,omitempty"`
}
Comment on lines +91 to +95
if b.hasFilesCheckError(currentService.ProtoFilePath, againstService.ProtoFilePath) {
issue := getServiceMovedIssue(currentService, againstService)
res = append(res, issue)
}

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.

3 participants