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

feat(workspace): make types camel case #4368

Merged
merged 8 commits into from
Oct 25, 2024
Merged

Conversation

ematipico
Copy link
Member

Summary

Closes #4366

All fields are now camel case.

This will break the playground, which means that we need to update it once next is merged to main

Test Plan

I updated the JS APIs, CI should pass.

@github-actions github-actions bot added A-CLI Area: CLI A-Core Area: core A-Project Area: project A-Linter Area: linter A-Parser Area: parser A-Formatter Area: formatter L-JavaScript Language: JavaScript and super languages L-CSS Language: CSS L-JSON Language: JSON and super languages A-Diagnostic Area: diagnostocis labels Oct 23, 2024
Copy link

codspeed-hq bot commented Oct 23, 2024

CodSpeed Performance Report

Merging #4368 will improve performances by 6.22%

Comparing feat/rename-workspace-fields (459a883) with next (f5f38dc)

Summary

⚡ 1 improvements
✅ 100 untouched benchmarks

Benchmarks breakdown

Benchmark next feat/rename-workspace-fields Change
pure_9395922602181450299.css[cached] 3.6 ms 3.4 ms +6.22%

@Conaclos
Copy link
Member

Should we also ensure that option values are in camel case? For now there are in Pascal Case in the playground.

@ematipico
Copy link
Member Author

Should we also ensure that option values are in camel case? For now there are in Pascal Case in the playground.

That's what I have been doing with this PR. Are there other options that I missed?

@Conaclos
Copy link
Member

That's what I have been doing with this PR. Are there other options that I missed?

I mean the values, not the names. For examples tabs instead of Tabs. I think we have to change the Display implementation?

@ematipico
Copy link
Member Author

That's what I have been doing with this PR. Are there other options that I missed?

I mean the values, not the names. For examples tabs instead of Tabs. I think we have to change the Display implementation?

Yes, that's what the PR is for :) so, if you spot something that I missed, please do so!

@Conaclos
Copy link
Member

Conaclos commented Oct 23, 2024

Should we not update Display implementation such as

impl Display for IndentStyle {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
IndentStyle::Tab => std::write!(f, "Tab"),
IndentStyle::Space => std::write!(f, "Space"),
}
}
}

?

To something like:

impl Display for IndentStyle {
    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
        match self {
            IndentStyle::Tab => std::write!(f, "tab"),
            IndentStyle::Space => std::write!(f, "space"),
        }
    }
}

@ematipico
Copy link
Member Author

The display implementation isn't used in WASM, so we don't need to change it

@ematipico ematipico merged commit 70542df into next Oct 25, 2024
11 of 12 checks passed
@ematipico ematipico deleted the feat/rename-workspace-fields branch October 25, 2024 12:17
ematipico added a commit that referenced this pull request Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-CLI Area: CLI A-Core Area: core A-Diagnostic Area: diagnostocis A-Formatter Area: formatter A-Linter Area: linter A-Parser Area: parser A-Project Area: project L-CSS Language: CSS L-JavaScript Language: JavaScript and super languages L-JSON Language: JSON and super languages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BREAKING: change workspace fields camel case e.g. skipped_diagnostics -> skippedDiagnostics
3 participants