Skip to content

Organize files under src/fsharp #13118

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

Merged
merged 44 commits into from
May 10, 2022
Merged

Organize files under src/fsharp #13118

merged 44 commits into from
May 10, 2022

Conversation

dsyme
Copy link
Contributor

@dsyme dsyme commented May 9, 2022

This builds on #13113 and is draft until that is in

This organises the source files under src/fsharp into various sub-directories.

@dsyme dsyme marked this pull request as draft May 9, 2022 11:37
@vzarytovskii
Copy link
Member

After this is merged, we'll need to update/rebase open PRs.

@dsyme dsyme marked this pull request as ready for review May 9, 2022 14:27
@dsyme dsyme changed the title [WIP] Organize files under src/fsharp Organize files under src/fsharp May 9, 2022
@dsyme
Copy link
Contributor Author

dsyme commented May 9, 2022

This is close to ready (once green). It's a fairly simple move-and-organize.

  • Utilities - general utilities
  • Facilities - compiler-specific facilities such as diagnostics sink
  • AbstractIL - what it says
  • SyntaxTree - parsing, lexing, syntax tree
  • TypedTree - the typed tree and basic ops on it
  • Checking - everything to do with checking
  • Optimize - everything to do with optimization and lowering
  • CodeGen - IlxGen, and ILX Erase
  • Driver - putting it all together
  • Symbols - as before
  • Service - as before
  • Legacy - what it says

However the list of sub-directories mixes "build" and "source" and we may want to fix that, see below

image

@vzarytovskii
Copy link
Member

  • Utilities - general utilities
  • Facilities - compiler-specific facilities such as diagnostics sink
  • AbstractIL - what it says
  • SyntaxTree - parsing, lexing, syntax tree
  • TypedTree - the typed tree and basic ops on it
  • Checking - everything to do with checking
  • Optimize - everything to do with optimization and lowering
  • CodeGen - IlxGen, and ILX Erase
  • Driver - putting it all together
  • Symbols - as before
  • Service - as before
  • Legacy - what it says

I would put this list to the docs somewhere.

@baronfel
Copy link
Member

baronfel commented May 9, 2022

Another useful convention I've seen is README.md documents in the root of each folder as a guide to the purpose/intent of the contents of that folder. Perhaps overkill to have one in every folder, but localized READMEs also lend themselves to nicer browsing experiences in github, since a README in a directory is rendered automatically when viewing the code.

@vzarytovskii
Copy link
Member

Another useful convention I've seen is README.md documents in the root of each folder as a guide to the purpose/intent of the contents of that folder. Perhaps overkill to have one in every folder, but localized READMEs also lend themselves to nicer browsing experiences in github, since a README in a directory is rendered automatically when viewing the code.

I actually really like the idea, but don't know how to make sure it's always up to date (other than having a checklist in PR template, which will be asking devs to make sure changes are reflected there).

@Lanayx
Copy link
Contributor

Lanayx commented May 9, 2022

Facilities and Utilities have too close meaning IMO

@dsyme
Copy link
Contributor Author

dsyme commented May 9, 2022

I have revamped this. src/fsharp is removed and at src level we have this:

image

Inside the Compiler directory we have this:

image

I made a decision to call the directory src\Compiler and not src\FSharp.Compiler.Service . This is because

  1. Typing the long name each time I look for a compiler file frankly gives me PTSD every time I type it. It just doesn't work for me. It's ok for FSharp.Build and the others, but not this component.
  2. Several of the files in Compiler are used in other components as well
  3. The vast majority of the important logic in the repo is in that directory (as it was in src\fsharp) and giving it the shorter name is useful.

@KevinRansom
Copy link
Contributor

Looks like a fine proposal, it's gonna mess with my autopilot, but I expect I will survive :-)

@dsyme
Copy link
Contributor Author

dsyme commented May 9, 2022

It's gonna mess with my autopilot, but I expect I will survive :-)

Totally. src\fsharp is hardwired into my fingers and brain. I'm literally never going to get over this.

@dsyme
Copy link
Contributor Author

dsyme commented May 9, 2022

DEVGUIDE.md should be updated as well, path to the FCS project in the benchmark guide.

I think I've got everything. I did a walk over the docs and made some adjustments, and added a guide to the directories to docs/index.md (linked prominently in README.md and DEVGUIDE.md)

@dsyme dsyme merged commit 1134707 into dotnet:main May 10, 2022
@smoothdeveloper
Copy link
Contributor

Pretty much like what I am seeing, and concur that readme.md files are nices (in the test infrastructure, there are some).

Even if some info is outdated, it remains better than tacit knowledge (of some) and good reminder of this knowledge to all, and easy enough to update.

Hope it won't be a pain to update the pending PR's, but this refactoring work has to get in, in short turn arounds 🙂, adjusting PRs takes a separate route.

@kerams
Copy link
Contributor

kerams commented May 10, 2022

Merging was smooth. Didn't have to manually resolve any conflicts in my PRs.

charlesroddie pushed a commit to charlesroddie/fsharp that referenced this pull request May 2, 2023
* cleanup

* split files

* rename

* split infos.fs and SymbolHelpres.fs

* split infos.fs and SymbolHelpres.fs

* fix code formating

* rename autobox --> LowerLocalMutables

* adjust names

* block --> ImmutableArray

* format

* Error --> SRDiagnostic

* Error --> SRDiagnostic

* this -> _

* rename and cleanup

* rename Diagnostic --> FormattedDiagnostic

* format sigs

* format sigs

* organise files in src/fsharp

* organise files in src/fsharp

* fix build

* fix build

* fix build

* merge

* move more files

* move more files

* move more files

* move more files

* fix build

* file reorg

* finish moves

* fix links in docs

* code format

* fix build

* save xlf, InteractiveSession/ --> Interactive/

* moved xlf

* cleanup .gitignore

* fixed Linux build

* cleanup docs

* cleanup docs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants