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

Change the name of a few variables #842

Closed
3 of 4 tasks
ghost opened this issue Jun 2, 2018 · 7 comments
Closed
3 of 4 tasks

Change the name of a few variables #842

ghost opened this issue Jun 2, 2018 · 7 comments
Assignees
Milestone

Comments

@ghost
Copy link

ghost commented Jun 2, 2018

The names and casing of variables are not very consistent. I propose to use lowercase for all variables such as OCAML and additionally do the following renaming:

  • %{path:file} --> %{dep:file}, I think this way it's clearer what it means
  • %{path-no-dep:file} --> %{path:file}
  • SCOPE_ROOT --> project_root
  • remove %{findlib:...} which is redundant with %{lib:...}

#576 adds a dir_kind in the code that tells us whether the directory contains a jbuild or dune file. We can use that to use the new names only in dune files

@ghost ghost added this to the 1.0.0 milestone Jun 2, 2018
@ghost ghost self-assigned this Jun 4, 2018
@ghost ghost assigned emillon and unassigned ghost Jun 28, 2018
@emillon
Copy link
Collaborator

emillon commented Jun 29, 2018

We can also use the ~syntax argument passed to the expansion functions (added in #887).

@ghost
Copy link
Author

ghost commented Jun 29, 2018

Indeed, it's the same kind of change

@rgrinberg
Copy link
Member

rgrinberg commented Jul 2, 2018

@emillon which parts of this PR are you tackling?

Btw, the listing of variables to be renamed is incomplete in this PR. Are you planning on tackling all the renames, or just those in this issue?

@emillon
Copy link
Collaborator

emillon commented Jul 2, 2018

I have no precise plans yet, but I was thinking about the path vs deps ones. Maybe we can close this and add bugs for individual items? That'll be easier to track & coordinate.

@rgrinberg
Copy link
Member

Up to you. I can handle the renames that aren't mentioned in this ticket for now.

@emillon
Copy link
Collaborator

emillon commented Jul 2, 2018

OK, I'll take care of the ones mentioned in here then.

emillon added a commit to emillon/dune that referenced this issue Jul 2, 2018
- `${path:file}` -> `%{dep:file}`
- `${path-no-dep:file}` -> `%{path:file}`

See ocaml#842

Signed-off-by: Etienne Millon <etienne@cryptosense.com>
emillon added a commit to emillon/dune that referenced this issue Jul 2, 2018
- `${path:file}` -> `%{dep:file}`
- `${path-no-dep:file}` -> `%{path:file}`

See ocaml#842

Signed-off-by: Etienne Millon <etienne@cryptosense.com>
emillon added a commit to emillon/dune that referenced this issue Jul 3, 2018
- `${path:file}` -> `%{dep:file}`
- `${path-no-dep:file}` -> `%{path:file}`

See ocaml#842

Signed-off-by: Etienne Millon <etienne@cryptosense.com>
emillon added a commit to emillon/dune that referenced this issue Jul 3, 2018
- `${path:file}` -> `%{dep:file}`
- `${path-no-dep:file}` -> `%{path:file}`

See ocaml#842

Signed-off-by: Etienne Millon <etienne@cryptosense.com>
emillon added a commit to emillon/dune that referenced this issue Jul 3, 2018
See ocaml#842

Signed-off-by: Etienne Millon <etienne@cryptosense.com>
emillon added a commit to emillon/dune that referenced this issue Jul 3, 2018
See ocaml#842

Signed-off-by: Etienne Millon <etienne@cryptosense.com>
@ghost
Copy link
Author

ghost commented Jul 8, 2018

This is done

@ghost ghost closed this as completed Jul 8, 2018
This issue was closed.
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

2 participants