search

agoda-com / Kakao

This repo is no longer supported. Please visit a https://github.com/KakaoCup/Kakao
Apache License 2.0
1.11k stars 102 forks source link

Wrong @DslMarker annotations #125

Closed v-rodionov closed 5 years ago

v-rodionov commented 5 years ago

Hi guys! I have some problem

Right now i can write something like that

cardsRecyclerView {
    cardsRecyclerView {
        cardsRecyclerView {
            hasDescendant {
                hasDescendant {
                    hasDescendant {  }
                }
            }
        }
    }
}

This code is totally wrong The construction does not make sense (the list in the same list in the same list checks something there and 2 more levels of checks). This is impossible to create in a real ui

This is because the kakao dsl used different @DskMarker annotations I think you should choose one marker annotation for all dsl and the problem will gone

Unlimity commented 5 years ago

Hi! Thank you for submitting the issue! I will look into it and see how we can improve. Maybe I misunderstood the Kotlin docs and samples on this topic

Unlimity commented 5 years ago

I revisited the Kotlin docs and you're right. Scope is getting limited only if enclosing receivers are marked with the same DslMarker annotation. We will fix that right after big refactor PR is merged. Thanks again!

matzuk commented 5 years ago

Hi, Ilya! @Unlimity You wrote that your team is working on big refactoring. Can you share info what refactoring you are doing? We have some minds about the development of Kakao. Maybe we will sync?

cdsap commented 5 years ago

HI @matzuk, this is one of the PR recently merged https://github.com/agoda-com/Kakao/pull/124

matzuk commented 5 years ago

@cdsap Thanks! And what else? Is there some roadmap of refactoring? Can @Ulman95 do PR to Kakao? Or he needs to wait for other changes?

cdsap commented 5 years ago

@Unlimity can you confirm please ?

Unlimity commented 5 years ago

@matzuk sure. Right now, the only changes left to make before 2.0 release is fix this dsl marker bug, upgrade dependencies and Kotlin version. Let's see what @Ulman95 got for us 👍