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

Sort plot-schema and add test to track plot-schema changes #5776

Merged
merged 18 commits into from
Jul 7, 2021

Conversation

archmoj
Copy link
Contributor

@archmoj archmoj commented Jun 28, 2021

Resolves #5588.
In PRs after this when there is a change in attributes (e.g. to implement a new feature), one should simply run

npm run schema

and commit the test/plot-schema.json file.

On CircleCI, that schema file would be tested against the auto-generated baseline at dist/plot-schema.json.
In the case of any difference a diff is presented and test should fail.
We also added a message on CircleCI asking contributors to run npm run schema and commit the results in the "test" folder (i.e. similar to baseline changes).

This would help track the changes to the API way before release cycles, right when a feature/documentation PR is submitted or even before.

In addition to above,

  1. As a result of this PR the plot-schema object is sorted to be as stable as possible for future comparisons.
  2. The compress_attributes browserify transform is now disabled to generate plotly.js files in the build folder which not only decreases the build & rebuild times but also help make this possible.

@plotly/plotly_js

@archmoj archmoj added this to the NEXT milestone Jun 28, 2021
@archmoj archmoj changed the title Test and track plot-schema changes Sort plot-schema and add test to track plot-schema changes Jun 28, 2021
@archmoj archmoj modified the milestones: NEXT, v2.3.0 Jun 28, 2021
Co-authored-by: Alex Johnson <johnson.alex.c@gmail.com>
for(var i = 0; i < allKeys.length; i++) {
var key = allKeys[i];
var v = obj[key];
newObj[key] = (typeof v === 'object' && v !== null && !(v instanceof Array)) ?
Copy link
Collaborator

Choose a reason for hiding this comment

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

Actually we need to dig into arrays too, right? We've got stuff in the schema like:

      "items": [
       {
        "valType": "number",
        "min": 0,
        "max": 1,
        "editType": "calc"
       },
       {
        "valType": "number",
        "min": 0,
        "max": 1,
        "editType": "calc"
       }
      ],

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call.
aea3022 also fixed cases where arrays e.g. enumerated valType turned into objects!
Before:

   "values": {
    "0": "forward",
    "1": "reverse"
   }

After

   "values": [
    "forward",
    "reverse"
   ]

 - ensure identical number of lines present in sorted schema
@archmoj archmoj requested a review from alexcjohnson June 30, 2021 18:14
Copy link
Collaborator

@alexcjohnson alexcjohnson left a comment

Choose a reason for hiding this comment

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

💃

Co-authored-by: Alex Johnson <johnson.alex.c@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature something new
Projects
None yet
Development

Successfully merging this pull request may close these issues.

commit plot-schema like a baseline?
2 participants