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

Convert all projects to use Mage #9842

Closed
wants to merge 18 commits into from

Conversation

andrewkroh
Copy link
Member

@andrewkroh andrewkroh commented Jan 2, 2019

Convert all projects to use Mage https://magefile.org for building.

  • Each Beat is buildable from both the OSS directory and the X-Pack directory.
    • This will make it simpler to add x-pack specific content in the future if desired.
  • beat-dashboards.zip will include the all dashboards (include x-pack).
  • Use the same logic to generate include/list.go in each Beat.
  • Generate a fields.go for each Beat/Module/Input/Monitor.
  • Generate the Travis config based on what the root magefile.go specifies to build/test.
  • Remove some unused scripts and files.
  • Update jenkins scripts to execute mage for building/testing.
  • Add a mage docs target that generates docs and opens the browser when PREVIEW is set.

TODO:

  • Update all projects to use mage.BeatProjectType.
  • Do we want to have the all the various config files checked in?
  • Fix metricbeat's system module docs to remove Go template stuff from the config.
  • Add docs to the root magefile.go.
  • Test APM
  • Fix the generators so they work on any OS.
  • Add a fields.go file to libbeat and remove libbeat content from each individual beat's include/fields.go.
  • Add something to aggregate code coverage reports.
  • Remove all the lingering requirements.txt files and use a single one.
  • Remove code that existed only to transition between make/mage.
  • Continue to remove duplication.
  • Find out what targets we are missing from users.
  • Add some basic docs for the build system (how to do X).

@andrewkroh andrewkroh added the in progress Pull request is currently in progress. label Jan 2, 2019
@andrewkroh andrewkroh requested a review from a team as a code owner January 2, 2019 09:57
})
}

func (Test) Integ() error {

Choose a reason for hiding this comment

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

exported method Test.Integ should have comment or be unexported

})
}

func (Test) Unit() error {

Choose a reason for hiding this comment

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

exported method Test.Unit should have comment or be unexported


type Test mg.Namespace

func (Test) All() error {

Choose a reason for hiding this comment

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

exported method Test.All should have comment or be unexported

})
}

type Test mg.Namespace

Choose a reason for hiding this comment

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

exported type Test should have comment or be unexported

Stage string
}

type Package mg.Namespace

Choose a reason for hiding this comment

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

exported type Package should have comment or be unexported

"update:fields",
}

func (Check) Targets() error {

Choose a reason for hiding this comment

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

exported method Check.Targets should have comment or be unexported

@@ -94,7 +172,372 @@ func addLicenseHeaders() error {
)
}

func (Check) Vet() error {

Choose a reason for hiding this comment

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

exported method Check.Vet should have comment or be unexported


type Check mg.Namespace

// Check checks that code is formatted and generated files are up-to-date.

Choose a reason for hiding this comment

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

comment on exported method Check.All should be of the form "All ..."

return mage.Clean(paths)
}

type Check mg.Namespace

Choose a reason for hiding this comment

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

exported type Check should have comment or be unexported

OutputFile: "build/distributions/dashboards/{{.Name}}-{{.Version}}{{if .Snapshot}}-SNAPSHOT{{end}}",
// --- Targets ---

func Clean() error {

Choose a reason for hiding this comment

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

exported function Clean should have comment or be unexported

}

// BuildGoDaemon builds the go-daemon binary (use crossBuildGoDaemon).
func BuildGoDaemon() error {

Choose a reason for hiding this comment

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

func name will be used as build.BuildGoDaemon by other packages, and that stutters; consider calling this GoDaemon

// TODO: Add generators.
}

Aliases = map[string]interface{}{

Choose a reason for hiding this comment

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

exported var Aliases should have comment or be unexported

@@ -6,6 +6,6 @@ REM
REM After running this once you may invoke mage.exe directly.

WHERE mage
IF %ERRORLEVEL% NEQ 0 go install github.com/ph/functionbeat/vendor/github.com/magefile/mage
IF %ERRORLEVEL% NEQ 0 go install github.com/elastic/beats/vendor/github.com/magefile/mage
Copy link
Contributor

Choose a reason for hiding this comment

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

🤦‍♂️

Copy link
Member

@ruflin ruflin left a comment

Choose a reason for hiding this comment

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

This change is great and I can't wait to get all this in. As usual to I will ask if we could split this up into multiple smaller PR's? I think there are quite a few changes in there that could happen separately to get it in quickly and make it more reviewable.

I hope most magefile commands could already be introduce by calling mage from the make commands, for example for the docs and this could happen in separate PR's?

- name: check
- name: test
- name: crosscompile
if: type != pull_request
Copy link
Member

Choose a reason for hiding this comment

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

What is the reason you do not want to have this on each PR?

@@ -1,3 +1,4 @@
---
Copy link
Member

Choose a reason for hiding this comment

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

Could all the changes to k8s go into a separate PR?

@@ -0,0 +1,146 @@
# DO NOT EDIT - AUTO-GENERATED
Copy link
Member

Choose a reason for hiding this comment

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

Very nice. I hope we can also use this later on to generate the Jenkins Pipeline file or that the Pipeline file actually contains all this logic.

Copy link
Member

Choose a reason for hiding this comment

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

Could this change also happen without mage?

@@ -0,0 +1,35 @@
// Licensed to Elasticsearch B.V. under one or more contributor
Copy link
Member

Choose a reason for hiding this comment

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

Separate PR for changes in heartbeat?

@urso
Copy link

urso commented Feb 28, 2020

Hi @andrewkroh, this PR seems to be stalled and quite out of date. Can we close this one?

@urso urso added the Stalled label Feb 28, 2020
@andrewkroh andrewkroh closed this Mar 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in progress Pull request is currently in progress. Stalled
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants