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

Replace generate_imports python script with mage imports #14498

Merged
merged 13 commits into from
Nov 14, 2019

Conversation

ChrsMark
Copy link
Member

@ChrsMark ChrsMark commented Nov 13, 2019

This PR tries to refactor and unify the way we create include/list.go files. This came after we moved light modules to OSS and while trying to create a light module under OSS. In that case this module will be included in list.go file despite the fact that it has no .go files. This was not a problem when light modules where under x-pack because there the list.go was created by mage and the script was taking into consideration the existence of .go files in a module so as to decide if it should be included in list.go.

So far we had:

This PR refactors the OSS part so as to make use of the mage respective target likewise x-pack.

cc: @jsoriano , @exekias

Copy link
Member

@jsoriano jsoriano left a comment

Choose a reason for hiding this comment

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

Yet another python script in the way to be removed 😃

metricbeat/Makefile Show resolved Hide resolved
dev-tools/mage/fields.go Outdated Show resolved Hide resolved
dev-tools/mage/fields.go Outdated Show resolved Hide resolved
@jsoriano jsoriano requested a review from kvch November 13, 2019 12:59
@ChrsMark ChrsMark requested a review from a team as a code owner November 13, 2019 14:45
dev-tools/mage/fields.go Show resolved Hide resolved
dev-tools/mage/fields.go Show resolved Hide resolved
dev-tools/mage/fields.go Show resolved Hide resolved
dev-tools/mage/fields.go Show resolved Hide resolved
Signed-off-by: chrismark <[email protected]>
Signed-off-by: chrismark <[email protected]>
Signed-off-by: chrismark <[email protected]>
if err != nil {
return errors.Wrap(err, "error gettting current directory")
}
return sh.Run("ln", "-sf", filepath.Join(pwd, vendorPath, "elastic/beats/metricbeat/scripts/generate_imports_helper.py"), filepath.Join(pwd, vendorPath, "elastic/beats/script/generate_imports_helper.py"))
Copy link
Member Author

Choose a reason for hiding this comment

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

This is sth which requires a careful look, not sure if it's ok to remove. Maybe @fearful-symmetry can comment here.

Copy link
Contributor

Choose a reason for hiding this comment

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

The new go generator has its own code for linking that, so I'm not sure what's using this?

func LinkImportsHelper() error {
vendorPath := "./vendor/github.com/"
pwd, err := os.Getwd()
if err != nil {
return errors.Wrap(err, "error gettting current directory")
}
return sh.Run("ln", "-sf", filepath.Join(pwd, vendorPath, "elastic/beats/metricbeat/scripts/generate_imports_helper.py"), filepath.Join(pwd, vendorPath, "elastic/beats/script/generate_imports_helper.py"))
}

Signed-off-by: chrismark <[email protected]>
@ChrsMark ChrsMark changed the title Change Metricbeat Makefile to use mage imports Replace generate_imports python Script with mage imports Nov 14, 2019
@ChrsMark ChrsMark changed the title Replace generate_imports python Script with mage imports Replace generate_imports python script with mage imports Nov 14, 2019
@ChrsMark ChrsMark merged commit 6b46c29 into elastic:master Nov 14, 2019
@ChrsMark ChrsMark added needs_backport PR is waiting to be backported to other branches. v7.6.0 labels Nov 14, 2019
ChrsMark added a commit to ChrsMark/beats that referenced this pull request Nov 14, 2019
@ChrsMark ChrsMark removed the needs_backport PR is waiting to be backported to other branches. label Nov 14, 2019
ChrsMark added a commit that referenced this pull request Nov 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants