-
Notifications
You must be signed in to change notification settings - Fork 1
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
Clean up code base #19
Comments
Specifically we could take one of two approaches: 1) Use diffing directly from the style specThe code in However, that function is already exported from the style spec. Instead of our edited version of this file, it would be much cleaner and more legible (not to mention easier to stay up to date spec-wise) if we used the exported function and only wrote code in this repo that makes that output more usable and is clear about the structure of that response before (from the differ) and after. import { diff } from '@mapbox/mapbox-gl-style-spec';
const formatOutput = (output) => {
...
}
const diffStyles = (a,b) => {
... add some definitions of what this output looks like
let output = diff(a,b);
return formatOutput(output);
} 2) Write the diff code from scratchThe entire structure of the We could write this (either based on Mapbox GL style recurse could be a helpful tool for us with this approach to check style properties. Layer additions/removals, and root level changes are already easy for us to diff without much code. Moving forwardI don't have a strong opinion on which approach we take, but I probably lean towards 2 just because it reflects the level of simplicity/complexity actually involved in what this repo does. It may depend a little on the future of this repo and what else we might use this for. |
As mentioned in #18 (comment) and followup from an early PR: #10
This code feels pretty crufty since the original code was borrowed for Mapbox GL JS code to set properties and then we appended it for our purposes. This mostly works, but makes further development difficult since the code is not at all clean.
Ideally we can clean this code up to be more understandable and (probably as follow up) test it.
The text was updated successfully, but these errors were encountered: