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

refr(enso-org/cloud-v2#1088): Make Project fields camelCase #9653

Merged
merged 2 commits into from
Apr 11, 2024

Conversation

indiv0
Copy link
Contributor

@indiv0 indiv0 commented Apr 8, 2024

Modifies fields of Project-like structs to be camelCase. This brings more of the backend API into line with the existing style guide.

Note that the changes in this commit are breaking if the IDE is run against a backend that hasn't been updated to account for the corresponding API changes.

Pull Request Description

Important Notes

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

  • The documentation has been updated, if necessary.
  • Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
  • All code follows the
    Scala,
    Java,
    and
    Rust
    style guides. In case you are using a language not listed above, follow the Rust style guide.
  • All code has been tested:
    • Unit tests have been written where possible.
    • If GUI codebase was changed, the GUI was tested when built using ./run ide build.

Modifies fields of `Project`-like structs to be `camelCase`. This brings
more of the backend API into line with the existing style guide.

Note that the changes in this commit are breaking if the IDE is run
against a backend that hasn't been updated to account for the
corresponding API changes.
@indiv0 indiv0 added the CI: No changelog needed Do not require a changelog entry for this PR. label Apr 8, 2024
Copy link
Contributor

@somebody1234 somebody1234 left a comment

Choose a reason for hiding this comment

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

just renames.
did not do QA of course - as of writing the corresponding backend PR is still open

},
packageName: 'Project_root',
/* eslint-disable @typescript-eslint/naming-convention */
Copy link
Contributor

Choose a reason for hiding this comment

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

prefer two separet eslint-disable-next-lines, that way we don't risk forgetting to re-enable it

// eslint-disable-next-line @typescript-eslint/naming-convention, @typescript-eslint/no-unused-vars
const { opened_by, ...newProjectState2 } = newProjectState
// eslint-disable-next-line @typescript-eslint/no-unused-vars
const { openedBy, ...newProjectState2 } = newProjectState
Copy link
Contributor

Choose a reason for hiding this comment

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

i guess we should change this to use the object.omit, so that it no longer needs the lint disable:

export function omit<T, Ks extends readonly (string & keyof T)[] | []>(

... on that note it seems like there's a typo there. oh well... i might fix that at some point in the future

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is the Ks the typo? What should it be?

Copy link
Contributor

Choose a reason for hiding this comment

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

no the typo is in the docs, which say "runtie array" 😅

Ks is short for keys, maybe it should be renamed too though

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Typo fixed :)

@PabloBuchu
Copy link
Contributor

qa 🟢

@PabloBuchu PabloBuchu added the CI: Ready to merge This PR is eligible for automatic merge label Apr 11, 2024
@mergify mergify bot merged commit f80e005 into develop Apr 11, 2024
37 checks passed
@mergify mergify bot deleted the wip/np/camel-case-project-state-1088 branch April 11, 2024 20:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: No changelog needed Do not require a changelog entry for this PR. CI: Ready to merge This PR is eligible for automatic merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants