Convert to snake case#8456
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #8456 +/- ##
==========================================
- Coverage 60.93% 60.90% -0.04%
==========================================
Files 374 374
Lines 54039 54084 +45
==========================================
+ Hits 32930 32941 +11
- Misses 21109 21143 +34
🚀 New features to boost your workflow:
|
rescript
@rescript/darwin-arm64
@rescript/darwin-x64
@rescript/linux-arm64
@rescript/linux-x64
@rescript/runtime
@rescript/win32-x64
commit: |
|
Oh no, sorry, I was too slow in reviewing , this has conflicts already. 😞 Could you rebase? |
|
@cknitt we should rename variants names? See PR description |
|
Rename the variants, but there are exceptions such as third-party library variants ( |
|
I don't think it's a good idea to rename some variants (parsetree.ml, etc). I will revert |
|
Out of interest, why are we renaming everything? Is it a new style guide? Seems to me like this touches so much maybe camel case should be the canonical choice. |
|
OCaml style guide is snake case. Only |
Ah I see. The changes to the compiler are the call-sites 👍🏼 |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 10e5e9e16b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| @@ -1,14 +1,14 @@ | |||
| (executable | |||
| (name ReactiveTest) | |||
| (name reactive_test) | |||
There was a problem hiding this comment.
When someone runs make test from analysis/reactive, the local Makefile still builds and executes test/ReactiveTest.exe, but this stanza now names the executable reactive_test, so Dune's target is test/reactive_test.exe and the documented local test command for this package fails until the Makefile is updated as well.
Useful? React with 👍 / 👎.
| module Config = GenTypeConfig | ||
| module String_map = Map.Make (String) | ||
| module String_set = Set.Make (String) | ||
| module Config = Gen_type_config |
There was a problem hiding this comment.
Probably better to use gentype_ instead of gen_type_ everywhere.
|
@codex review |
|
Codex Review: Didn't find any major issues. You're on a roll. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
| else Lsp.Uri.to_string t | ||
|
|
||
| (* Light weight, hopefully-enough-for-the-purpose fn to encode URI components. | ||
| Built to handle the reserved characters listed in | ||
| https://en.wikipedia.org/wiki/Percent-encoding. Note that this function is not | ||
| general purpose, rather it's currently only for URL encoding the argument list | ||
| passed to command links in markdown. *) | ||
| let encodeURIComponent text = | ||
| let encode_u_r_i_component text = |
|
Codex found some more: I found two weird acronym splits introduced by this branch:
Likely intended names:
I also saw these suspicious-looking names, but they were already present at
|
3 participants
Renamed variant names (also expection names), some follow pascal case format, but in OCaml it's generally snake case (first letter capitalized, of course).Close #8442