Skip to content

Conversation

@shmsr
Copy link
Member

@shmsr shmsr commented Jun 9, 2024

Proposed commit message

After talking to @cmacknz, decided to go with the fork and update the dependencies of the same as well. Now, the new input will use github.com/elastic/go-sfdc@master.

See: https://github.com/elastic/go-sfdc

Use the upstream dependency directly instead of the forked version, as the tip is the same i.e., use github.com/g8rswimmer/go-sfdc instead of github.com/elastic/go-sfdc.

A while ago, someone reported internally at Slack that they having issues pulling dependencies when using GoLand, although we never encountered this problem, and neither did the CI. We currently use replace directives because we initially planned to fork the repository to add new features. However, we do not have a requirement to add new features at the moment.

Instead of forking the repository under the elastic organization, we can directly use the upstream repository, as the tip of the master branch is the same. This change will remove the unnecessary replace directives and potentially resolve any issues people may face when using GoLand, etc.

Update: Now it is clear why apm-server was breaking and not beats. apm-server uses beats as a dependency. As the replace directive inside beats' go.mod has no version, it applies to replace to all versions of github.com/g8rswimmer/go-sfdc to point to github.com/g8rswimmer/go-sfdc@master. But now that apm-server is the main module, beats is not; replace directive is not supported. See this. As the replace is not respected, it is not able to find the particular version of the dependency as: go mod edit -replace=github.com/g8rswimmer/go-sfdc=github.com/elastic/go-sfdc@master was used that had no version on LHS. But replace also used no version; so it applied to all versions (irrespective of what's written there).

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works
  • I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

Related

@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Jun 9, 2024
@mergify
Copy link
Contributor

mergify bot commented Jun 9, 2024

This pull request does not have a backport label.
If this is a bug or security fix, could you label this PR @shmsr? 🙏.
For such, you'll need to label your PR with:

  • The upcoming major version of the Elastic Stack
  • The upcoming minor version of the Elastic Stack (if you're not pushing a breaking change)

To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

  • backport-v8./d.0 is the label to automatically backport to the 8./d branch. /d is the digit

@mergify mergify bot assigned shmsr Jun 9, 2024
@shmsr shmsr requested a review from kush-elastic June 10, 2024 05:49
@shmsr shmsr marked this pull request as ready for review June 20, 2024 16:07
@shmsr shmsr requested a review from a team as a code owner June 20, 2024 16:07
@shmsr shmsr added the Team:Service-Integrations Label for the Service Integrations team label Jun 20, 2024
@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Jun 20, 2024
@shmsr shmsr requested a review from a team as a code owner June 21, 2024 06:53
@shmsr shmsr changed the title x-pack/filebeat/input/salesforce: Use upstream dependency directly x-pack/filebeat/input/salesforce: Use github.com/elastic/go-sfdc dependency directly Jun 21, 2024
@pierrehilbert
Copy link
Contributor

@lalit-satapathy we are missing a review from someone in your team here

Copy link
Member

@ishleenk17 ishleenk17 left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Team:Service-Integrations Label for the Service Integrations team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants