Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[crosslink tool] Implement crosslink tool #8822

Merged
merged 19 commits into from
Apr 4, 2022

Conversation

bryan-aguilar
Copy link
Contributor

Description: This PR adds the crosslink tool to the repository. Crosslink will automatically insert any missing replace statements for intra-repository go modules. For more info please refer to the crosslink readme here.

Currently crosslink is run using the make crosslink target. It is setup to only perform non-destrructive actions as replace statements should be additive after initial crosslink on-boarding. If other operations are required such as overwriting or pruning crosslink can be run with those flags separately.

After running make crosslink a missing replace statement in windowperfcountersreceiver was inserted.

I have also ran crosslink with the --overwrite flag and commited the changes to the go.mod files in awscloudwatchlogsexporter, awsemfexporter and awsprometheusremotewriteexporter as an example of what changes crosslink would make. There are many more go.mod files in this repository that could be updated. I did not want to create an initial PR with all of those changes at once though. If this is accepted and merged I can update the rest of the go.mod files and create a follow on PR.

I did not add the execution of crosslink to any other process of the makefile. I was hoping to have a discussion on whether that should be done now or should we leave it isolated so developers can use it as they wish? Typically crosslink should be ran from the repository root to ensure that all submodules are mapped properly.

Link to tracking Issue: #7055

Testing: Testing for crosslink can be found in the opentelemetry-go-build-tools repository.

Documentation:

Makefile Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
bryan-aguilar and others added 2 commits March 23, 2022 15:49
Co-authored-by: Anthony Mirabella <a9@aneurysm9.com>
@TylerHelmuth
Copy link
Member

Is there any value in adding some docs around using this new capability, maybe in CONTRIBUTING.md? The goal being making it easier for contributors (especially new contributors) to about make crosslink. Or is this command something that would only be run in our actions?

@bryan-aguilar
Copy link
Contributor Author

Is there any value in adding some docs around using this new capability, maybe in CONTRIBUTING.md? The goal being making it easier for contributors (especially new contributors) to about make crosslink. Or is this command something that would only be run in our actions?

This is a good callout and something I have been thinking about myself. I think there is definitely value in adding it somewhere. I believe CONTRIBUTING.MD has instructions to insert replace statements. Let me review that and add some context to those docs.

@bryan-aguilar
Copy link
Contributor Author

Updated CONTRIBUTING.md General Recommendations section to include crosslink.

@codeboten codeboten added the ready to merge Code review completed; ready to merge by maintainers label Mar 30, 2022
@jpkrohling
Copy link
Member

PR rebased for 0.48.0.

@bogdandrutu
Copy link
Member

@bryan-aguilar please fix CI/CD

@bogdandrutu bogdandrutu removed the ready to merge Code review completed; ready to merge by maintainers label Apr 4, 2022
@bryan-aguilar
Copy link
Contributor Author

@bogdandrutu fixed, sorry about the delay.

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.

6 participants