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

Removing remove directive #703

Open
clarus opened this issue May 17, 2019 · 3 comments
Open

Removing remove directive #703

clarus opened this issue May 17, 2019 · 3 comments

Comments

@clarus
Copy link
Contributor

clarus commented May 17, 2019

In many packages there are the directives:

remove: ["rm" "-R" "%{lib}%/coq/user-contrib/Semantics"]
flags: light-uninstall

However, with opam 2, these are not necessary any more. In many PR reviews it was advised to remove them. Is there a policy about these directives? Should we remove all of them?

I would be in favor of:

  • advising not to use these directives;
  • making a huge PR to clean all the existing remove / light-uninstall instances at once.
@palmskog
Copy link
Contributor

I'm definitely in favor of advising people not to use them. However, for the "huge PR", I would defer to @gares if that's advisable with CI. Maybe CI could be circumvented for the PR?

@palmskog
Copy link
Contributor

palmskog commented May 17, 2019

Another aspect of this removal that should be mentioned: a lot of knowledge about logical paths will be lost. Ideally, one would process and remove:

remove: ["rm" "-R" "%{lib}%/coq/user-contrib/Semantics"]

and at the same time add logpath:Semantics in tags (if not already present).

@gares
Copy link
Member

gares commented May 18, 2019

Indeed it would be nice to turn these remove instructions into a logpath metadata. It is a good idea. I'm afraid I did not advise package authors to do that, but we should.

Wrt the pr, I'm ok with it if it turns the fields into a logpath metadata. But beware il will be a lot of work, while it will naturally happen if we are strict with pr reviews. Maybe we should just improve the linting check.

erikmd added a commit to erikmd/math-comp that referenced this issue Oct 15, 2019
CohenCyril pushed a commit to math-comp/math-comp that referenced this issue Oct 18, 2019
* feat: Add build for mathcomp/mathcomp-dev:coq-8.10

* fix(coq-mathcomp-ssreflect.opam): Bump coq upper bound

* fix(*.opam): Remove "remove" directive

href: coq/opam#703
JasonGross added a commit to JasonGross/opam-coq-archive that referenced this issue Oct 21, 2019
For packages I'm maintaining.

c.f. coq#703
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

No branches or pull requests

3 participants