Skip to content

Convert all projects to use Mage#9842

Closed
andrewkroh wants to merge 18 commits intoelastic:masterfrom
andrewkroh:feature/mage-refactor
Closed

Convert all projects to use Mage#9842
andrewkroh wants to merge 18 commits intoelastic:masterfrom
andrewkroh:feature/mage-refactor

Conversation

@andrewkroh
Copy link
Copy Markdown
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
magefile.go Outdated
Copy link
Copy Markdown

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

magefile.go Outdated
Copy link
Copy Markdown

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

magefile.go Outdated
Copy link
Copy Markdown

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

magefile.go Outdated
Copy link
Copy Markdown

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

magefile.go Outdated
Copy link
Copy Markdown

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

magefile.go Outdated
Copy link
Copy Markdown

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

magefile.go Outdated
Copy link
Copy Markdown

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

magefile.go Outdated
Copy link
Copy Markdown

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 ..."

magefile.go Outdated
Copy link
Copy Markdown

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

magefile.go Outdated
Copy link
Copy Markdown

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

@andrewkroh andrewkroh force-pushed the feature/mage-refactor branch from 8e72c60 to c3a6acb Compare January 2, 2019 10:02
}

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

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{}{
Copy link
Copy Markdown

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


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
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🤦‍♂️

Copy link
Copy Markdown
Contributor

@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
Copy Markdown
Contributor

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
Copy Markdown
Contributor

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
Copy Markdown
Contributor

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
Copy Markdown
Contributor

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
Copy Markdown
Contributor

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
Copy Markdown

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