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

Binding implementation to multiple interfaces #652

Merged
merged 9 commits into from Nov 15, 2019

Conversation

VladPodilnyk
Copy link
Contributor

@VladPodilnyk VladPodilnyk commented Sep 8, 2019

This commit is actually ported version of the following PR:
478820a
made by Yaroslav Sahach (#528).

@VladPodilnyk
Copy link
Contributor Author

Things to do:

  • add more tests.
  • add .named, .tagged support

@codecov-io
Copy link

codecov-io commented Sep 8, 2019

Codecov Report

Merging #652 into develop will decrease coverage by 0.01%.
The diff coverage is 54.83%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #652      +/-   ##
===========================================
- Coverage    75.03%   75.02%   -0.02%     
===========================================
  Files          532      532              
  Lines        13116    13147      +31     
  Branches       642      660      +18     
===========================================
+ Hits          9842     9863      +21     
- Misses        3274     3284      +10
Impacted Files Coverage Δ
...la/izumi/distage/model/definition/LocatorDef.scala 73.68% <0%> (-13.82%) ⬇️
...cala/izumi/distage/model/definition/Bindings.scala 52.38% <100%> (+2.38%) ⬆️
...e/model/definition/dsl/AbstractBindingDefDSL.scala 83.33% <50%> (-14.29%) ⬇️
...mi/distage/model/definition/dsl/ModuleDefDSL.scala 71.27% <77.77%> (+4.21%) ⬆️
...mentals/platform/language/SourceFilePosition.scala 100% <0%> (+50%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e1f82b2...80a14be. Read the comment docs.

@VladPodilnyk VladPodilnyk force-pushed the feature/517-binding-to-multiple-interfaces branch from 71fa98b to 6102b10 Compare September 18, 2019 15:17
Copy link
Member

@neko-kai neko-kai left a comment

Choose a reason for hiding this comment

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

@VladPodilnyk do you think yoo could add a test that uses .named/.tagged in this PR?

@VladPodilnyk VladPodilnyk force-pushed the feature/517-binding-to-multiple-interfaces branch from 6102b10 to 497a769 Compare September 24, 2019 16:12
@VladPodilnyk
Copy link
Contributor Author

@neko-kai, how much time do I have? or you want to procced by yourself?

@neko-kai
Copy link
Member

@VladPodilnyk There's no rush, just checking!

@VladPodilnyk
Copy link
Contributor Author

@neko-kai, ok, I was just asking, because I'm a little bit slow... but if there is time, I would like to proceed.

Copy link
Member

@neko-kai neko-kai left a comment

Choose a reason for hiding this comment

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

Almost there, just a few corrections before merge 👍

@VladPodilnyk VladPodilnyk force-pushed the feature/517-binding-to-multiple-interfaces branch from 97e48ee to fc27f01 Compare November 14, 2019 23:44
@neko-kai neko-kai merged commit cf8ba95 into develop Nov 15, 2019
@VladPodilnyk
Copy link
Contributor Author

@neko-kai thanks a lot for your patience and support with this PR!

@neko-kai
Copy link
Member

@VladPodilnyk Nice! Congratulations 🚀

@neko-kai neko-kai deleted the feature/517-binding-to-multiple-interfaces branch November 15, 2019 00:16
@VladPodilnyk
Copy link
Contributor Author

😊, @neko-kai could I approach another one? I can take anything from https://github.com/7mind/izumi/milestone/9 or maybe you have smth particular in mind?

@neko-kai
Copy link
Member

@VladPodilnyk
There's a bunch of issues marked "good first issue" - https://github.com/7mind/izumi/issues?q=is%3Aissue+is%3Aopen+label%3A%22good+first+issue%22

Of these I'd say the best to do is LogStage: add support for strings with .stripMargin – it's medium-hard difficulty, but really useful if it works because we can start using multi-line strings in logs properly then.

@VladPodilnyk
Copy link
Contributor Author

then I'll take it (#684), ok?

neko-kai pushed a commit that referenced this pull request Nov 15, 2019
* Binding implementation to multiple interfaces

This commit is actually ported version of the following PR:
478820a
made by Yaroslav Sahach.

* fix relationship between types.

* wip: playing around.

* wip...

* .to[] and .to[](name) - works.

* rework implementation.

* ready to review.

* apply suggestions.

* pattern match inside multiple bind interpreter.
@neko-kai
Copy link
Member

@VladPodilnyk Sure! Thank you!

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.

None yet

4 participants