search

magic-lang / rock

ooc compiler written in ooc
http://ooc-lang.org/
MIT License
14 stars 4 forks source link

Running `make clean` from `make safe`. #43

Closed davidpiuva closed 8 years ago

davidpiuva commented 8 years ago

So that you don't have to write make clean && make safe.

vendethiel commented 8 years ago

Why not add "clean" as a dependency instead?

davidpiuva commented 8 years ago

This is my first pull request on Rock. How would I do that?

alexnask commented 8 years ago

Instead of doing make clean, just change it up to: 

rescue: download-bootstrap clean
    $(MAKE) clean bootstrap
alexnask commented 8 years ago

Btw, make rescue already cleans ($(MAKE) clean bootstrap) but it could be useful for make safe :)

davidpiuva commented 8 years ago

I have no idea what that means. Just wanted to get rid of the extra command.

alexnask commented 8 years ago

Sure, Makefile syntax is pretty confusing.

Just revert back the change to rescue and change the safe rule to: 

safe:   boehmgc clean
    OOC='bin/safe_rock' $(MAKE) self

And I think you are all set.

marcusnaslund commented 8 years ago

I feel I should object to PRs like these. Of course, my divine power does not extend to this repo and the decision belongs to @thomasfanell and @fredrikbryntesson , but we shouldn't let this repo diverge from the ooc-lang/rock repo unneccessarily.

I think our makefile should do what the original makefile does, unless there's a good reason. This does not qualify. If @davidpiuva wants to shorten a double command it would take 20 seconds to set up a bash alias for that.

Of course, I'm also OK if this change is also allowed into the other repo.

alexnask commented 8 years ago

I think this change would be accepted into ooc-lang, it seems pretty logical (of course it would be more consistent if make self always cleaned, not only make safe).

Anyways, I kind of think the opposite on this issue, small changes like these that only affect the workflow a bit and not the resulting compiler do not personally bother me that much.

Changes that would bother me (if no one bothered to open PR's for ooc-lang/rock) would be bugfixes etc.

I think the two repo's should only diverge on additional features for magic, anything that is related to the existing functionality should be common (I can now see your argument the way I phrased this :P)

marcusnaslund commented 8 years ago

@shamanas I guess we have almost the same viewpoint just expressed in different ways. :) I see now that the line was already modified once (the boehmgc dependency is not in the makefile on ooc-lang/rock ). I can make a PR with either clean or both boehmgc clean to upstream, depending on which is best there.

alexnask commented 8 years ago

I don't see why boehmgc would be a dependency, since it only needs to be compiled once.
clean is fine though, I would accept that upstream :)

EDIT: I should specify I mean that boehmgc should be already built by rescue, if safe was the first ran command I would see why it was there.

marcusnaslund commented 8 years ago

Well then, a PR has been made upstream. :)