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

Add lib to Makefile #344

Merged
merged 5 commits into from
Apr 13, 2020
Merged

Add lib to Makefile #344

merged 5 commits into from
Apr 13, 2020

Conversation

straight-shoota
Copy link
Member

No description provided.

Makefile Outdated Show resolved Hide resolved
@RX14
Copy link
Contributor

RX14 commented Apr 7, 2020

I'm less sure of this now, I think the behaviour should be this:

  • the makefile should warn if lib doesn't exist
  • the makefile should provide a bootstrap target
  • the warning should contain instructions to either run shards or make bootstrap.

a good bootstrap for the makefile is:

MOLINILLO_VERSION = $(shell $(CRYSTAL) eval 'require "yaml"; puts YAML.parse(File.read("shard.lock"))["shards"]["molinillo"]["version"]')
MOLINILLO_URL = "https://github.com/crystal-lang/crystal-molinillo/archive/v$(MOLINILLO_VERSION).tar.gz"

with the target

bootstrap:
	mkdir -p lib/molinillo
	curl -L $(MOLINILLO_URL) | tar -xz -C lib/molinillo --strip-components=1

@straight-shoota
Copy link
Member Author

@RX14 I've just enhanced the recipe for lib with bootstrap as a fallback if shards install fails. That way it should work without having to run a second command.
If you want to force the boostrap behaviour, you can run make lib SHARDS=false.

@@ -17,7 +17,7 @@ commands:
- crystal/shards-install
- run: make
- run: make test
- crystal/format-check
- run: crystal tool format --check src spec
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this required? It really shouldn't be.

Copy link
Member Author

Choose a reason for hiding this comment

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

See commit message on 4d559ac

tmp/integrations is created by integration specs and contains non-formatted files

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe crystal/format-check (or crystal tool format) should only apply to spec src by default. But this is also a very specific problem that shards contains ill-formatted files in its tree after the test have run.

Alternatives would be to run format check before make test or clean tmp/integrations afterwards.

@RX14
Copy link
Contributor

RX14 commented Apr 7, 2020

I think this is fine

@j8r
Copy link
Contributor

j8r commented Apr 10, 2020

Is it possible to move forward? Updating the Alpine Linux shards package requires bootstrapping to get molinillo.

@straight-shoota
Copy link
Member Author

Rebased and squashed.

@waj waj merged commit 5be28ac into crystal-lang:master Apr 13, 2020
@straight-shoota straight-shoota deleted the makefile branch April 13, 2020 18:56
@bcardiff bcardiff added this to the v0.11.0 milestone May 27, 2020
f-fr pushed a commit to f-fr/shards that referenced this pull request Jan 2, 2021
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.

5 participants