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

refactor: simplify reading dashboard config from a json file #1828

Merged
merged 4 commits into from
Oct 6, 2021

Conversation

damianstasik
Copy link
Contributor

@damianstasik damianstasik commented Oct 4, 2021

New Pull Request Checklist

Issue Description

I replaced json-file-plus with a simple file reading function and JSON.parse. json-file-plus is no longer maintained, the last update was 4 years ago. There is also no need for JSON manipulation, which made this refactor much easier. After the package was gone there was no longer a need for the data property nesting, so I flattened the structure by one level.

To improve readability of the changes I suggest to hide whitespace changes.

Approach

TODOs before merging

  • Add tests
  • Add changes to documentation (guides, repository pages, in-code descriptions)
  • A changelog entry is created automatically using the pull request title (do not manually add a changelog entry)

@parse-github-assistant
Copy link

parse-github-assistant bot commented Oct 4, 2021

Thanks for opening this pull request!

  • ❌ Please check all required checkboxes at the top, otherwise your pull request will be closed.

  • ⚠️ Remember that a security vulnerability must only be reported confidentially, see our Security Policy. If you are not sure whether the issue is a security vulnerability, the safest way is to treat it as such and submit it confidentially to us for evaluation.

@mtrezza
Copy link
Member

mtrezza commented Oct 4, 2021

there was no longer a need for the data property nesting, so I flattened the structure by one level.

This is only internal, without any changes required for existing deployments? In other words, this does not lead to a breaking change?

@damianstasik
Copy link
Contributor Author

@mtrezza Fortunately it is limited only to this file, but just in case we can restore the nesting and tackle it in the future if needed.

@mtrezza
Copy link
Member

mtrezza commented Oct 5, 2021

I think we can keep the reduced nesting of this PR, if we are sure it is only internal. But I see you already reverted it, so we can also go that way 🙂

@mtrezza mtrezza merged commit 921712a into parse-community:alpha Oct 6, 2021
@damianstasik damianstasik deleted the remove-json-file-plus branch October 6, 2021 10:52
mtrezza added a commit that referenced this pull request Oct 6, 2021
* refactor: simplify reading dashboard config from a json file (#1828)

* refactor: simplify reading config from a json file

* refactor: restore `data` nesting of config object

Co-authored-by: Manuel <5673677+mtrezza@users.noreply.github.com>

* docs: fix changelog branch names (#1837)

* docs: reword changelog quote

* docs: fix release changelog filename

Co-authored-by: Damian Stasik <visualfanatic@users.noreply.github.com>
mtrezza added a commit that referenced this pull request Oct 6, 2021
* refactor: simplify reading dashboard config from a json file (#1828)

* refactor: simplify reading config from a json file

* refactor: restore `data` nesting of config object

Co-authored-by: Manuel <5673677+mtrezza@users.noreply.github.com>

* docs: fix changelog branch names (#1837)

* docs: reword changelog quote

* docs: fix release changelog filename

Co-authored-by: Damian Stasik <visualfanatic@users.noreply.github.com>

Co-authored-by: Damian Stasik <visualfanatic@users.noreply.github.com>
@parseplatformorg
Copy link
Contributor

🎉 This pull request has been released in version 3.2.1-alpha.1

@parse-github-assistant
Copy link

The label state:released on @alpha cannot be used here.

mtrezza added a commit to mtrezza/parse-dashboard that referenced this pull request Oct 8, 2021
* alpha:
  chore(release): 3.2.1-alpha.1 [skip ci]
  fix: enabling context menu for read-only cells (parse-community#1844)
  docs: add info about --dev parameter (parse-community#1842)
  docs: fix release changelog filename
  docs: reword changelog quote
  docs: fix changelog branch names (parse-community#1837)
  refactor: simplify reading dashboard config from a json file (parse-community#1828)
@parseplatformorg
Copy link
Contributor

🎉 This pull request has been released in version 3.2.1-beta.1

@parseplatformorg parseplatformorg added the state:released-beta Released as beta version label Oct 8, 2021
@parseplatformorg
Copy link
Contributor

🎉 This pull request has been released in version 3.2.1

@parseplatformorg parseplatformorg added the state:released Released as stable version label Oct 8, 2021
@mtrezza mtrezza added the state:released-alpha Released as alpha version label Oct 8, 2021
beiguancyc pushed a commit to beiguancyc/parse-dashboard that referenced this pull request Oct 9, 2021
* source: (53 commits)
  chore(release): 3.2.1 [skip ci]
  chore(release): 3.2.1-beta.1 [skip ci]
  ci: fix prerelease labels
  chore(release): 3.2.1-alpha.1 [skip ci]
  fix: enabling context menu for read-only cells (parse-community#1844)
  docs: add info about --dev parameter (parse-community#1842)
  build: merge beta (parse-community#1841)
  build: merge alpha (parse-community#1840)
  docs: fix release changelog filename
  docs: reword changelog quote
  docs: fix changelog branch names (parse-community#1837)
  refactor: simplify reading dashboard config from a json file (parse-community#1828)
  ci: update release branch names
  chore(release): 3.2.0 [skip ci]
  feat: add ctrl + arrow key to jump to first/last cell; fix left/right key navigation with hidden columns (parse-community#1827)
  refactor: upgrade inquirer from 8.1.2 to 8.1.3 (parse-community#1829)
  refactor: upgrade otpauth from 7.0.5 to 7.0.6 (parse-community#1830)
  refactor: replace create-react-class with ES6 classes (parse-community#1818)
  refactor: replace query-string with URLSearchParams (parse-community#1819)
  docs: fix typo in refactor changelog entry
  ...

# Conflicts:
#	package-lock.json
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
state:released Released as stable version state:released-alpha Released as alpha version state:released-beta Released as beta version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants