Closed deepakgarg08 closed 5 years ago
Merging #150 into master will increase coverage by
0.44%
. The diff coverage isn/a
.
@@ Coverage Diff @@
## master #150 +/- ##
============================================
+ Coverage 43.24% 43.69% +0.44%
- Complexity 155 160 +5
============================================
Files 49 49
Lines 1274 1284 +10
Branches 139 139
============================================
+ Hits 551 561 +10
Misses 684 684
Partials 39 39
Impacted Files | Coverage Δ | Complexity Δ | |
---|---|---|---|
.../java/upb/ida/provider/RiveScriptBeanProvider.java | 100% <0%> (ø) |
3% <0%> (+1%) |
:arrow_up: |
.../src/main/java/upb/ida/logging/LoggerProvider.java | 100% <0%> (ø) |
9% <0%> (+4%) |
:arrow_up: |
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 f1aaad4...558d4fc. Read the comment docs.
Obviously, I mentioned this in the comments. It can't be merged, I just tagged you for the task, because neither feat/springsecurity is not compiling, nor docker stuff is working. So, I wrong locally everything in seperate project and there I tested this. So, I uploaded all this here for you to review for feedback and what else we need to add/improve into this. I already discussed all this with Maqbool.
Deepak Garg
On Thu, 16 May 2019, 14:01 Clemens Damke, notifications@github.com wrote:
@Cortys requested changes on this pull request.
How does this relate to #139 https://github.com/dice-group/ida/issues/139? As far as I can tell, this implementation does not migrate the existing user management implementation. Our goal is to migrate user data storage from Neo4j to the RDF store, i.e. a reimplementation of the UserService https://github.com/dice-group/ida/blob/feat/springsecurity/ida-ws/src/main/java/upb/ida/service/UserService.java class and everything related to it is required (or something equivalent).
The user management implementation you provided in this PR is not integrated into ida-ws and as such not usable yet. Please integrate your user management code into IDA. The existing user admin page and the login screen should work with the new storage backend afterwards.
@aliazmi68 https://github.com/aliazmi68 currently works on the user management UI revamp #138 https://github.com/dice-group/ida/issues/138.
138 https://github.com/dice-group/ida/issues/138 should only affect
frontend code, #139 https://github.com/dice-group/ida/issues/139 should only affect backend code. Both task should thus ideally not interfere with each other. If they do however, please coordinate accordingly.
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/dice-group/ida/pull/150?email_source=notifications&email_token=AFSNP2P5SZFOBHTEB2SXSJTPVVEJLA5CNFSM4HNLL6VKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOBY2N6JI#pullrequestreview-238346021, or mute the thread https://github.com/notifications/unsubscribe-auth/AFSNP2NUX73LRURFDESQR5TPVVEJLANCNFSM4HNLL6VA .
On Thu, 16 May 2019, 14:01 Clemens Damke, notifications@github.com wrote:
@Cortys requested changes on this pull request.
How does this relate to #139 https://github.com/dice-group/ida/issues/139? As far as I can tell, this implementation does not migrate the existing user management implementation. Our goal is to migrate user data storage from Neo4j to the RDF store, i.e. a reimplementation of the UserService https://github.com/dice-group/ida/blob/feat/springsecurity/ida-ws/src/main/java/upb/ida/service/UserService.java class and everything related to it is required (or something equivalent).
The user management implementation you provided in this PR is not integrated into ida-ws and as such not usable yet. Please integrate your user management code into IDA. The existing user admin page and the login screen should work with the new storage backend afterwards.
@aliazmi68 https://github.com/aliazmi68 currently works on the user management UI revamp #138 https://github.com/dice-group/ida/issues/138.
138 https://github.com/dice-group/ida/issues/138 should only affect
frontend code, #139 https://github.com/dice-group/ida/issues/139 should only affect backend code. Both task should thus ideally not interfere with each other. If they do however, please coordinate accordingly.
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/dice-group/ida/pull/150?email_source=notifications&email_token=AFSNP2P5SZFOBHTEB2SXSJTPVVEJLA5CNFSM4HNLL6VKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOBY2N6JI#pullrequestreview-238346021, or mute the thread https://github.com/notifications/unsubscribe-auth/AFSNP2NUX73LRURFDESQR5TPVVEJLANCNFSM4HNLL6VA .
Hi Deepak this is really hard to review as there is so much other stuff in this PR. The thing is you must need to do this stuff in feat/spring-security
branch. And I am not getting why this stuff happening with you kindly remember we need this feature ASAP
this PR is not relevant he is working on rdf_user_management
It contains around 9 commits, 7 are referenced from other source