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

Merge fork with enhancements/fixes #456

Merged
merged 20 commits into from
Apr 24, 2023

Conversation

kalinon
Copy link
Contributor

@kalinon kalinon commented Sep 29, 2021

My fork has been getting a bit ahead so i wanted to open a PR with my working branch.

This includes the following bug fixes:

It also includes the following enhancements:

  • ameba recomendations
  • add retry to adapter base
  • add support for querying with Array(UUID)
  • add Relationship annotation that is applied to methods which are defined relationships

kalinon added 12 commits August 28, 2021 10:25
If a column is named `created_at` or `updated_at` is not a type `Time?`, then the following occurs:

```There was a problem expanding macro 'macro_4801706624'

Code in lib/granite/src/granite/transactions.cr:82:5

 82 | {% if @type.instance_vars.select { |ivar| ivar.annotation(Granite::Column) }.map(&.name.stringify).includes? "created_at" %}
      ^
Called macro defined in lib/granite/src/granite/transactions.cr:82:5

 82 | {% if @type.instance_vars.select { |ivar| ivar.annotation(Granite::Column) }.map(&.name.stringify).includes? "created_at" %}

Which expanded to:

 > 1 |
 > 2 |       if mode == :create
 > 3 |         @created_at = time.at_beginning_of_second
                                  ^---------------------
```

this simply adds a type check.
@kalinon kalinon changed the title Merge fork enhancements/fixes Merge fork with enhancements/fixes Sep 29, 2021
@SebastianSzturo
Copy link

Any chance to get these fixes merged?

@confact
Copy link

confact commented Apr 16, 2022

ping @drujensen - Could this be merged? It solves some of my issues :)

@confact
Copy link

confact commented Apr 20, 2022

@kalinon could you split this one up into smaller MR (one for each fix preferably) - So it is easier to review and maybe approve it?

@robacarp
Copy link
Member

related: #462

@kalinon
Copy link
Contributor Author

kalinon commented Apr 20, 2022

@confact i had already done that, but they were never merged. It soon grew and was hard to manage for me. #454, #455 are smaller pieces of this.

Copy link
Member

@crimson-knight crimson-knight left a comment

Choose a reason for hiding this comment

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

Thank you for this!

@crimson-knight crimson-knight merged commit 5996066 into amberframework:master Apr 24, 2023
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