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

[flake]: Introduce flake-parts, treefmt, and other changes #284

Merged
merged 15 commits into from Feb 14, 2023
Merged

[flake]: Introduce flake-parts, treefmt, and other changes #284

merged 15 commits into from Feb 14, 2023

Conversation

huwaireb
Copy link
Contributor

@huwaireb huwaireb commented Feb 6, 2023

Tested on x86_64-linux

Note

There's no support for enabling submodules for flake top-level yet, as such

  1. Submodules have to be explicitly enabled somehow, mentioned in the README
  2. The coq-sarpi flake input needs to reflect the .gitmodule

@huwaireb

This comment was marked as outdated.

@Alizter
Copy link
Collaborator

Alizter commented Feb 6, 2023

Hmm it seems you got rid of the OCaml format detection and also ocaml lsp. Can you add those back in?

Also see the doc on Nix in contributing. It used to be in the readme but it was moved.

@Alizter
Copy link
Collaborator

Alizter commented Feb 6, 2023

We should not be formatting the submodules. Can you ignore them?

.github/workflows/build.yml Outdated Show resolved Hide resolved
.gitignore Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
Comment on lines +77 to +79
This currently only applies to building the default package (coq-lsp), which is the server.
Clients don't have specific submodules as of yet.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Rather than submodules, I would say shells. I mainly wanted the flake for development however if we wish to distribute OCaml pacakges relying on Coq libraries the situation is a bit complicated. We also want people to be able to override the serapi and coq dependencies and avoid using submodules. This is not easy to do from what I have tried.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't believe submodules and shells are referring to the same thing here?

We also want people to be able to override the serapi and coq dependencies and avoid using submodules. This is not easy to do from what I have tried.

I couldn't even get it to build without coq vendored in with submodules, overriding serapi worked fine, but coq wasn't even building.

- **Coq Platform** (coming soon)
- [Do it yourself!](#server-1)

#### **Visual Studio Code**:
- Official Marketplace: https://marketplace.visualstudio.com/items?itemName=ejgallego.coq-lsp
- Open VSX: https://open-vsx.org/extension/ejgallego/coq-lsp
- Nix:
```nix
inputs.coq-lsp = { type = "git"; url = "https://github.com/ejgallego/coq-lsp.git"; submodules = true; };
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do you prefer the full url over the github: ones?

Copy link
Contributor Author

@huwaireb huwaireb Feb 8, 2023

Choose a reason for hiding this comment

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

Passing in ?submodules=1 or ?submodules=true with github: failed in my case,

@DieracDelta
Copy link

DieracDelta commented Feb 8, 2023

Hello! I tested this on aarch64-darwin. I'm able to build and run the coq-lsp binary by building the default target.

flake.nix Outdated Show resolved Hide resolved
@Alizter Alizter merged commit b6f245b into ejgallego:main Feb 14, 2023
@ejgallego ejgallego added this to the 0.1.5 milestone Mar 6, 2023
@ejgallego ejgallego added kind: enhancement New feature or request part: build labels Mar 6, 2023
- run: npm ci
- run: npx prettier -c .
- name: 📐 Format with alejandra, ocamlformat, prettier
run: nix fmt
Copy link
Owner

Choose a reason for hiding this comment

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

This seems broken, see #449

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#452

This formats the code, I was supposed to use the check output target as opposed to running the formatter.

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

Successfully merging this pull request may close these issues.

None yet

4 participants