Skip to content

Conversation

addaleax
Copy link
Collaborator

As part of this:

  • Rename the analytics config writer part of the build package
    and add information other than the Segment API key to it
  • Clean up some functions in the build package to just take
    a config argument instead of a list of arguments with names
    that differ between call site and function definition (this
    was super confusing before, and still is in a lot of places)
  • Make sure that the compiled binary identifies itself as such
    in the evergreen script

addaleax added 3 commits July 15, 2021 17:27
As part of this:

- Rename the analytics config writer part of the build package
  and add information other than the Segment API key to it
- Clean up some functions in the build package to just take
  a config argument instead of a list of arguments with names
  that differ between call site and function definition (this
  was super confusing before, and still is in a lot of places)
- Make sure that the compiled binary identifies itself as such
  in the evergreen script
@addaleax addaleax marked this pull request as ready for review July 15, 2021 15:59
outDir: path.dirname(bundleOutputFile),
outFile: path.basename(bundleOutputFile),
const bundler = new Bundler(config.input, {
outDir: path.dirname(config.execInput),
Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I remember being confused by the name execInput here as the outFile for the bundle. Maybe we can rename execInput to bundleFile inside config? (no need to do in that PR)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I’m very much up for renaming a number of config options :)

@addaleax addaleax merged commit 384a15d into main Jul 16, 2021
@addaleax addaleax deleted the 896-dev branch July 16, 2021 08:02
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.

2 participants