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.

Sign up for GitHub

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

Jump to bottom

Metaquot: Embed errors in the AST instead of raising #397

Merged
merged 10 commits into from
Mar 22, 2023
Merged

Metaquot: Embed errors in the AST instead of raising #397

merged 10 commits into from
Mar 22, 2023

Conversation

Burnleydev1
Copy link
Contributor

No description provided.

@Burnleydev1 Burnleydev1 mentioned this pull request Mar 15, 2023
Closed
@Burnleydev1 Burnleydev1 changed the title Metaquot: Embed errors in the AST Metaquot: Embed errors in the AST instead of raising Mar 15, 2023
panglesd
panglesd requested changes Mar 15, 2023
View reviewed changes
Copy link
Collaborator

@panglesd panglesd left a comment

Choose a reason for hiding this comment

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

You are getting closer!

But in your (draft) PR, there are still exceptions that are raised!

You also need to modify the Expr module.

metaquot/ppxlib_metaquot.ml Outdated Show resolved Hide resolved
@Burnleydev1 Burnleydev1 marked this pull request as ready for review March 16, 2023 07:50
@Burnleydev1 Burnleydev1 requested review from panglesd and removed request for pitag-ha and ceastlund March 16, 2023 07:53
@Burnleydev1
Copy link
Contributor Author

I'm sorry, Please @pitag-ha, @ceastlund, could you review the changes?

@ceastlund
Copy link
Collaborator

Looks pretty good. I think the two things missing are running dune fmt to re-ocamlformat the files, and signing off your commits. You can do that with git rebase --signoff.

@Burnleydev1
Copy link
Contributor Author

Thanks @ceastlund, I just implemented it now.

@Burnleydev1 Burnleydev1 force-pushed the embedded_errors branch 3 times, most recently from 65f3c45 to 9e3d669 Compare March 16, 2023 15:52
panglesd
panglesd approved these changes Mar 16, 2023
View reviewed changes
Copy link
Collaborator

@panglesd panglesd left a comment

Choose a reason for hiding this comment

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

Looks good, thanks @Burnleydev1!

metaquot/ppxlib_metaquot.ml Outdated Show resolved Hide resolved
@panglesd
Copy link
Collaborator

Could you add an entry to the changelog describing what you've done? Maybe focus on the user implications: multiple errors are now reported in metaquot!

@Burnleydev1 @panglesd
multiple errors are now reported in metaquot
79e1f81 
Co-authored-by: panglesd <[email protected]>
Signed-off-by: Burnleydev1 <[email protected]>
@Burnleydev1
Copy link
Contributor Author

Thanks @panglesd, I have updated the changelog message.

@panglesd
Copy link
Collaborator

Sorry, I meant can you update the file named CHANGES.md at the root of ppxlib sources (this one) to add an entry in the unreleased section?

Thanks!

@Burnleydev1
Copy link
Contributor Author

I'm sorry @panglesd, when I save my changes, all the ------------------ changes to ---, even though i did not change it.

@Burnleydev1
Copy link
Contributor Author

I am trying to fix it now

@Burnleydev1
Copy link
Contributor Author

Burnleydev1 commented Mar 21, 2023 •
edited
Loading

Hello @panglesd, I think I fixed it, it was VScode deleting the extra - whenever I tried saving, I used text edit to make my changes and I think it is fine now.

@panglesd panglesd merged commit f7035cd into ocaml-ppx:main Mar 22, 2023
@panglesd
Copy link
Collaborator

Thanks a lot for the contribution!

tmattio added a commit to tmattio/opam-repository that referenced this pull request Jun 20, 2023
@tmattio
[new release] ppxlib and ppxlib-bench (0.30.0)
76c31d4 
CHANGES:

- Adopt the OCaml Code of Conduct on the repo (ocaml-ppx/ppxlib#426, @pitag-ha)

- Clean up misleading attribute hints when declared for proper context. (ocaml-ppx/ppxlib#425, @ceastlund)

- Ast_pattern now has ebool, pbool helper, and a new map.(ocaml-ppx/ppxlib#402, @Burnleydev1)

- multiple errors are now reported in `metaquot`. (ocaml-ppx/ppxlib#397, @Burnleydev1)

- Add `Attribute.declare_with_attr_loc` (ocaml-ppx/ppxlib#396, @dvulakh)

- Add "ns" and "res" as reserved namespaces(ocaml-ppx/ppxlib#388, @davesnx)

- Make quoter `let` binding non-recursive (ocaml-ppx/ppxlib#401, @sim642)

- Fix failure of 'lift_map_with_context' in traverse by compile-time
  evaluation of 'fst' and 'snd' (ocaml-ppx/ppxlib#390, @smuenzel)

- Driver: Bias the mapping from magic to version towards the current version,
  as it is usually the common case and it helps when magic numbers are
  ambiguous (such as on development versions) (ocaml-ppx/ppxlib#409, @shym)

- Remove unnecessary test dependencies towards base and stdio (ocaml-ppx/ppxlib#421, @kit-ty-kate)

- Update description to reflect that `ppxlib` contains more than a library
  (ocaml-ppx/ppxlib#422, @pitag-ha)

- Add support for OCaml 5.1, excluding OCaml `5.1.0~alpha1` (ocaml-ppx/ppxlib#428, @shym, @Octachron , @pitag-ha, @panglesd)
- Driver: Fix `-locations-check` option for coercions with ground  (ocaml-ppx/ppxlib#428, @Octachron)
tmattio added a commit to tmattio/opam-repository that referenced this pull request Jun 20, 2023
@tmattio
[new release] ppxlib and ppxlib-bench (0.30.0)
5fe9af6 
CHANGES:

- Adopt the OCaml Code of Conduct on the repo (ocaml-ppx/ppxlib#426, @pitag-ha)

- Clean up misleading attribute hints when declared for proper context. (ocaml-ppx/ppxlib#425, @ceastlund)

- Ast_pattern now has ebool, pbool helper, and a new map.(ocaml-ppx/ppxlib#402, @Burnleydev1)

- multiple errors are now reported in `metaquot`. (ocaml-ppx/ppxlib#397, @Burnleydev1)

- Add `Attribute.declare_with_attr_loc` (ocaml-ppx/ppxlib#396, @dvulakh)

- Add "ns" and "res" as reserved namespaces(ocaml-ppx/ppxlib#388, @davesnx)

- Make quoter `let` binding non-recursive (ocaml-ppx/ppxlib#401, @sim642)

- Fix failure of 'lift_map_with_context' in traverse by compile-time
  evaluation of 'fst' and 'snd' (ocaml-ppx/ppxlib#390, @smuenzel)

- Driver: Bias the mapping from magic to version towards the current version,
  as it is usually the common case and it helps when magic numbers are
  ambiguous (such as on development versions) (ocaml-ppx/ppxlib#409, @shym)

- Remove unnecessary test dependencies towards base and stdio (ocaml-ppx/ppxlib#421, @kit-ty-kate)

- Update description to reflect that `ppxlib` contains more than a library
  (ocaml-ppx/ppxlib#422, @pitag-ha)

- Add support for OCaml 5.1, excluding OCaml `5.1.0~alpha1` (ocaml-ppx/ppxlib#428, @shym, @Octachron , @pitag-ha, @panglesd)
- Driver: Fix `-locations-check` option for coercions with ground  (ocaml-ppx/ppxlib#428, @Octachron)
@tmattio tmattio mentioned this pull request Jun 20, 2023
Merged
tmattio added a commit to tmattio/opam-repository that referenced this pull request Jun 20, 2023
@tmattio
[new release] ppxlib (0.30.0)
d22894c 
CHANGES:

- Adopt the OCaml Code of Conduct on the repo (ocaml-ppx/ppxlib#426, @pitag-ha)

- Clean up misleading attribute hints when declared for proper context. (ocaml-ppx/ppxlib#425, @ceastlund)

- Ast_pattern now has ebool, pbool helper, and a new map.(ocaml-ppx/ppxlib#402, @Burnleydev1)

- multiple errors are now reported in `metaquot`. (ocaml-ppx/ppxlib#397, @Burnleydev1)

- Add `Attribute.declare_with_attr_loc` (ocaml-ppx/ppxlib#396, @dvulakh)

- Add "ns" and "res" as reserved namespaces(ocaml-ppx/ppxlib#388, @davesnx)

- Make quoter `let` binding non-recursive (ocaml-ppx/ppxlib#401, @sim642)

- Fix failure of 'lift_map_with_context' in traverse by compile-time
  evaluation of 'fst' and 'snd' (ocaml-ppx/ppxlib#390, @smuenzel)

- Driver: Bias the mapping from magic to version towards the current version,
  as it is usually the common case and it helps when magic numbers are
  ambiguous (such as on development versions) (ocaml-ppx/ppxlib#409, @shym)

- Remove unnecessary test dependencies towards base and stdio (ocaml-ppx/ppxlib#421, @kit-ty-kate)

- Update description to reflect that `ppxlib` contains more than a library
  (ocaml-ppx/ppxlib#422, @pitag-ha)

- Add support for OCaml 5.1, excluding OCaml `5.1.0~alpha1` (ocaml-ppx/ppxlib#428, @shym, @Octachron , @pitag-ha, @panglesd)
- Driver: Fix `-locations-check` option for coercions with ground  (ocaml-ppx/ppxlib#428, @Octachron)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@panglesd panglesd panglesd approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Burnleydev1 @ceastlund @panglesd