Get rid of Makefile.nt in the yacc directory.#762
Conversation
|
Is this sensible to do piecemeal? I though other instances simply leave |
|
Thanks a lot for your interest in this, David.
My idea was to progressively get rid of all the Makefile.nt in the
subdirectories, so that one can finally also get rid of the main
Makefile.nt and have just one Makefile that works everywhere.
Regarding the fact of doing it piecemeal, it seems to me that,
technically speaking, the commits are rather independent from each
other. Also, I thought that having several (smaller and independent)
commits / PRs would make each of them easier to review, thus making the
integration of the whole thing easier.
But I may very well be wrong on these and am very open to any
suggestion.
|
|
I think it's fine to eliminate duplication directory by directory (this is how we have been proceeding for some time). The question from @dra27 (I think) is whether we want to leave a tiny Makefile.nt which only includes Makefile. And when all Makefile.nt have been made trivial this way, we get rid of all of them at once. The benefit of keeping Makefile.nt is that developers under Windows can simply do "make -f Makefile.nt" (as they are used to do) anywhere, instead of having to do either that or just "make" depending on the directory. |
|
Ah, got it, thanks.
I can certainly update the two PRs in this direction.
Is it okay to replace the calls to $(MAKEREC) by calls to $(MAKE) (in
the main Makefile.nt) right
now (as is done in the two PRs), or is it preferred to also defer
this until the actual elimination of the Makefile.nt files itself?
|
|
Btw, one thing remains unclear to me. In the two last commits by me that
have been merged, in the Author line, my full name has been replaced by
my git username. I don't understand how this happened because (I checked)
my original commits did have my full name in the Author line. Any idea on
this?
|
Let's ask our git expert. @gasche? |
I would have a slight preference for not doing it, in order to ensure that the Makefile.nt won't be removed too early by someone else who finds out that it is not used anymore. |
|
Alain Frisch (2016/08/24 03:01 -0700):
I get your point. I'm still unsure because not doing the replacement now |
|
Are you presently working on merging the entire build system (total elimination of Makefile.nt)? |
|
David Allsopp (2016/08/24 04:51 -0700):
Yes. Unless there is an objection agains it, it seems to me it would be Of course, any comment is more than welcome. |
|
Definitely no objection - that's awesome! It was on my list for post-OPAM, but no way I'd be getting to it before the middle of next year! If you need any help/testing (especially with the FlexDLL bootstrapping stuff), do ping me. Personally, I'd put all the changes through as one pull request (with lots of commits), but that's just a preference. |
|
Thanks a lot, David.
With my current strategy based on several PRs, I guess my main need
would be for somebody to review and merge the commits.
At the beginning, I really thought it was wiser to have several smaller
bullets which I hoped would be easier to review/swallow.
But now, if the community considers it is too much noise to review each
PR independently and tings its more coherent to have just one big PR
with many commits, I fine with that, too.
|
|
There are also things that are not strictly speaking part of merging
build systems, but I come across these things while trying to merge. So
far, my approach has been to do separate PRs to distinguish things from
each other and also to keep each thing as small and as easy to review as
possible.
I have to say, I slightly prefere seeing things being merged step by
step as compared to the "one PR" approach because it gives me the
feeling that things move forward, and also because it means that the
changes get a chance to be tested earlier.
But as I said before, if there is a consensus on the "one PR" approach,
I'll certainly do that.
|
|
If it'd helpful (it'd certainly be interesting for me!), can we have a chat about the directions for this? It's an area that I've been musing on a lot but, with other pressures, not done a huge amount with, and it'd be good to know where everyone else's thoughts are too! |
|
Of course we can talk about it, with pleasure.
By phone? On IRC? What would be your preference?
|
|
@dra27 Can you comment on whether you'd prefer to keep calling Makefile.nt (instead of just Makefile) from the root Makefile.nt for those subdirectories where the merger is done? |
|
Yes, cf. |
Considering that Makefile.nt is just an include of Makefile, this can hardly break. Considering @dra27 preference, I would suggest keeping the call to sub Makefile.nt files from the root one (i.e. use $(MAKEREC)). Can you apply the change, also to the other similar PR? |
yacc/Makefile has been changed so that it also works on Windows.
72f2224 to
bb18f35
Compare
|
Alain Frisch (2016/08/25 00:06 -0700):
Okay.
Sure. Both PRs have now been updated and rebased. Thanks a lot for your comments @alainfrisch and @dra27! |
yacc/Makefile has been changed so that it also works on Windows.
Remove naked pointer checker
The Makefile has been changed so that it also works on Windows.
As a consequence, in OCaml's main Makefile.nt, building things in the
yacc directory can now be done by invoking $(MAKE) rather than
$(MAKEREC).