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

Include JSON formatting tool binary in release. #38288

Merged
merged 1 commit into from
Feb 24, 2020

Conversation

damien
Copy link
Contributor

@damien damien commented Feb 24, 2020

Summary

SUMMARY: Infrastructure "Bundle JSON formatting tool with distribution"

Purpose of change

At present, the only way to use the JSON formatting tool is either via the website mentioned in doc/JSON_STYLE.md or by compiling the tool via make json_formatter.

I'd like to be able to run this tool outside the context of a browser or the C:DDA repo to do things like:

  1. Run the formatter as part of an editor integration
  2. Run the formatter within a CI step or Github action outside of the C:DDA repo
  3. Run the formatter bundled with an arbitrary version of C:DDA to determine if some JSON is compatible with that version (useful for doing things like git-bisect to figure out what version compat for a mod breaks)

Describe the solution

Add the JSON formatting tool binary to the list of files included in the release.

Describe alternatives you've considered

  1. Compiling the tool in another project. Seems like a waste of effort as each build compiles the formatter that's guaranteed to work for that exact version of C:DDA.
  2. Finding a different way to validate JSON. Likely a huge duplication of effort that isn't really required.
  3. Calling the XHR endpoint on http://dev.narc.ro/cataclysm/format.html . Aside from being a bit rude to do this without asking, I'd rather not decouple builds or scripts to something over a network.

Testing

This PR should be good if the build completes and the release includes the compiled binary in tools/format/json_formatter.(exe|cgi)

Additional context

First of a number of miscellaneous changes I'm hoping to make to make a nicer out-of-the-box contribution experience.

@damien damien requested a review from ZhilkinSerg February 24, 2020 04:08
@damien damien force-pushed the add-json-formatter-to-dist branch from 7d6cae6 to f29c9d2 Compare February 24, 2020 04:30
@ZhilkinSerg ZhilkinSerg added Code: Build Issues regarding different builds and build environments Organization General development organization issues labels Feb 24, 2020
@ZhilkinSerg ZhilkinSerg merged commit 7ddabba into CleverRaven:master Feb 24, 2020
@damien damien deleted the add-json-formatter-to-dist branch March 18, 2020 01:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code: Build Issues regarding different builds and build environments Organization General development organization issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants