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

Remove require-package label support #363

Merged
merged 3 commits into from
Dec 14, 2021
Merged

Remove require-package label support #363

merged 3 commits into from
Dec 14, 2021

Conversation

Leonidas-from-XIV
Copy link
Collaborator

This tests functionality that we don't support anymore since the way to do this is via the dune stanza.

@Leonidas-from-XIV Leonidas-from-XIV added this to the 2.0.0 milestone Dec 13, 2021
@NathanReb
Copy link
Contributor

Probably worth removing the label as well as part of this PR! It should probably have a mention in the changelog, maybe simply a ref to this PR in the remove ocaml-mdx rule entry would suffice. What do you think?

@Leonidas-from-XIV Leonidas-from-XIV changed the title Remove test for rule Remove require-package label support Dec 13, 2021
@Leonidas-from-XIV
Copy link
Collaborator Author

I've removed the label and added a changelog entry. I think it won't hurt to be specific about this in the changelog in case people wonder what happened to this label.

@NathanReb
Copy link
Contributor

To clarify this label was only consumed by ocaml-mdx rule. The only affected users would be the ones who use this label without using ocaml-mdx rule and they should probably be notified! I think it's alright to remove it from MDX in 2.0.0 without an explicit deprecation as it was directly tied to the rule subcommand which we did deprecate!

@@ -22,6 +22,8 @@
is 4.08 to 4.13 now (#345, @Leonidas-from-XIV)
- Do not install deprecated `mdx` binary anymore (#274, @gpetiot)
- Remove deprecated `rule` command (#312, @gpetiot)
- Remove support for `require-package` label, use the `mdx` stanza in dune
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe mention here that it was only used by MDX rule and that it can otherwise be safely removed!

@Leonidas-from-XIV Leonidas-from-XIV merged commit 0ca5d50 into realworldocaml:main Dec 14, 2021
@Leonidas-from-XIV Leonidas-from-XIV deleted the remove-stanza-package-test branch December 14, 2021 08:42
Leonidas-from-XIV added a commit to Leonidas-from-XIV/opam-repository that referenced this pull request Dec 14, 2021
CHANGES:

#### Added

- Add trailing `;;` to the output of toplevel phrases that were missing it.
  (realworldocaml/mdx#346, @Leonidas-from-XIV)
- Make MDX compatible with OCaml 4.14 (realworldocaml/mdx#356, @NathanReb)

#### Fixed

- Use the same output as the normal toplevel. Mdx used to carry an unsafe patch
  to work around a bug fixed in OCaml 4.06 and that patch would change the
  printed types in some corner cases. (realworldocaml/mdx#322, @emillon)

#### Removed

- Dropped compatibility with older OCaml versions. The minimal supported range
  is 4.08 to 4.13 now (realworldocaml/mdx#345, @Leonidas-from-XIV)
- Do not install deprecated `mdx` binary anymore (realworldocaml/mdx#274, @gpetiot)
- Remove deprecated `rule` command (realworldocaml/mdx#312, @gpetiot)
- Remove support for `require-package` label, use the `mdx` stanza in dune
  instead. This label was only used for the `rule` command and can now be
  safely removed. (realworldocaml/mdx#363, @Leonidas-from-XIV)
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.

2 participants