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

Update to Crystal/0.25.0 #857

Merged
merged 43 commits into from
Jun 20, 2018
Merged

Update to Crystal/0.25.0 #857

merged 43 commits into from
Jun 20, 2018

Conversation

robacarp
Copy link
Member

This is an internal branch copy of the branch @bcardiff opened in #826

@faustinoaq faustinoaq mentioned this pull request Jun 16, 2018
@robacarp
Copy link
Member Author

This is pretty close.

I've disabled automatic json parsing of json requests for now, I think that will need to be revisited and might even not be worth it for the framework to do.

A crystal spec produces this output, from build_spec:


Failures:

  1) building a generated app generates a binary
     Failure/Error: File.exists?("bin/#{TEST_APP_NAME}").should be_true

       Expected: true
            got: false

     # /Users/robert/Documents/repositories/amberframework/amber/spec/build_spec.cr:45

  2) building a generated app crystal spec can be executed
     Failure/Error: spec_result.should contain "Finished in"

       Expected:   ""
       to include: "Finished in"

     # /Users/robert/Documents/repositories/amberframework/amber/spec/build_spec.cr:55

Finished in 19.56 seconds
106 examples, 2 failures, 0 errors, 0 pending

Failed examples:

crystal spec /Users/robert/Documents/repositories/amberframework/amber/spec/build_spec.cr:44 # building a generated app generates a binary
crystal spec /Users/robert/Documents/repositories/amberframework/amber/spec/build_spec.cr:54 # building a generated app crystal spec can be executed

It seems like there are a few places in generated apps which need to be updated.

@robacarp
Copy link
Member Author

robacarp commented Jun 17, 2018

Status:

  • JSON params parse
  • Amber compiles
  • Specs pass
    • Build Spec passes
  • Generated app compiles
  • Dependent shards are all up to date

Shard Hacks Needing Resolving:

  • Citrine-i18n -> Techmagistar/i18n dependency chain needs fixed. Pull request Currently pointing to a fork kindly provided by @mamantoha
    • Citrine-i18n needs update PR, merge and release
  • Garnet-Spec -> ysbaddaden/selenium-webdriver-crystal dependency chain needs fixed. Pull request Currently pointing to a fork kindly provided by @paulcsmith
    • Garnet-Spec needs update PR, merge and release
  • Amber Router needs PR merge and release, Pull request
  • Granite needs PR merge and release, Pull request

Copy link
Contributor

@faustinoaq faustinoaq left a comment

Choose a reason for hiding this comment

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

Travis spec are failing due to a dependency issue:

Outdated shard.lock (wrong resolver). Please run shards update instead.

We had a similar issue here: #814 (comment)

@faustinoaq faustinoaq dismissed their stale review June 19, 2018 17:45

Dependency issues fixed (wrong citrine tag on shard.yml)

@robacarp robacarp force-pushed the crystal/0.25.0 branch 2 times, most recently from 12fa6fa to 54d5a31 Compare June 19, 2018 22:16
Copy link
Contributor

@faustinoaq faustinoaq left a comment

Choose a reason for hiding this comment

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

@robacarp Awesome, Thank you! 😄

@eliasjpr eliasjpr merged commit bb51aaf into master Jun 20, 2018
@faustinoaq faustinoaq deleted the crystal/0.25.0 branch June 20, 2018 00:38
@ilovezfs
Copy link

Hurray! @faustinoaq can we get a new tag please? 🙏

@faustinoaq
Copy link
Contributor

faustinoaq commented Jun 20, 2018

can we get a new tag please

@ilovezfs Amber team is already working on that 👍

We're trying to find critical bugs and fix them before a new release 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants