Skip to content

Attribute-based reverse mapping with SourceMemberAttribute - #3337

Merged
jbogard merged 2 commits into
LuckyPennySoftware:masterfrom
timmi-on-rails:issue-3129
Mar 19, 2020
Merged

Attribute-based reverse mapping with SourceMemberAttribute#3337
jbogard merged 2 commits into
LuckyPennySoftware:masterfrom
timmi-on-rails:issue-3129

Conversation

@timmi-on-rails

@timmi-on-railstimmi-on-rails commented Feb 26, 2020•
edited by lbargaoanu
Loading

Copy link
Copy Markdown
Contributor

Based on PR #3336
Added test and fix.

Fixes#3129 .
#3329

lbargaoanu
lbargaoanu reviewed Feb 27, 2020
View reviewed changes
Comment threadsrc/UnitTests/AttributeBasedMaps.cs Outdated
lbargaoanu
lbargaoanu reviewed Feb 27, 2020
View reviewed changes
Comment threadsrc/UnitTests/AttributeBasedMaps.cs Outdated
@lbargaoanulbargaoanu added this to the v.next milestone Feb 27, 2020
@timmi-on-rails

timmi-on-rails commented Feb 27, 2020

Copy link
Copy Markdown
ContributorAuthor

Can you please trigger CI again. There seems to be an issue not related to the sourcecode

@lbargaoanu

lbargaoanu commented Feb 28, 2020

Copy link
Copy Markdown
Contributor

It's ok, it will run again when you rebase after that other PR is merged.

@lbargaoanu

lbargaoanu commented Mar 9, 2020

Copy link
Copy Markdown
Contributor

@timmi-on-rails Can you please rebase on master?

@timmi-on-rails

timmi-on-rails commented Mar 11, 2020

Copy link
Copy Markdown
ContributorAuthor

I got a conflict, when I wanted to rebase on master.
Can you work with this here?

@lbargaoanu

lbargaoanu commented Mar 11, 2020

Copy link
Copy Markdown
Contributor

It's all your code. Surely you can solve any conflicts with your own code :)

@timmi-on-rails

timmi-on-rails commented Mar 11, 2020

Copy link
Copy Markdown
ContributorAuthor

Ok, I think I got it right now.

Doesn't rebasing and force pushing invalidate your code reviews, because I could make a mistake by accident (when resolving conflicts)?
I am inexperienced in rebasing across forks and extended git usage.

lbargaoanu
lbargaoanu approved these changes Mar 12, 2020
View reviewed changes

@lbargaoanulbargaoanu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks!

@lbargaoanu

lbargaoanu commented Mar 12, 2020

Copy link
Copy Markdown
Contributor

Yes, the idea is to rebase and review again. But all I'm looking the second time around is rebase errors, so it's quick. Thanks for taking the time to do everything properly. I'm sure it'll be easier next time :)

@jbogard
jbogard merged commit 0189c1e into LuckyPennySoftware:masterMar 19, 2020
@timmi-on-rails
timmi-on-rails deleted the issue-3129 branch March 19, 2020 13:44
@lock

lockBot commented Apr 19, 2020

Copy link
Copy Markdown

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@locklockBot locked as resolved and limited conversation to collaborators Apr 19, 2020
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@jbogardjbogardjbogard approved these changes
+1 more reviewer
@lbargaoanulbargaoanulbargaoanu approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@jbogardjbogard

Labels

Improvement

Projects

None yet

Milestone

10.0.0

Development

Successfully merging this pull request may close these issues.

Attribute-based Reverse Mapping with SourceMember does not work

3 participants

@timmi-on-rails@lbargaoanu@jbogard