For creating client specific detection points #35
Conversation
| @@ -10,8 +10,6 @@ If you are just wanting to get a demo going, see the [DemoSetup.md](DemoSetup.md | |||
|
|
|||
| This is the parent directory of several sample applications | |||
|
|
|||
| **Note: the applications in the sample-apps/ directory generally require java 1.8 or greater.** | |||
There was a problem hiding this comment.
@shreyasdn have a look at this - I added this after your fork. Can you pull in this change?
|
@shreyasdn This is a HUGE change for a first PR - nice work! If the comments seem overwhelming, let me know. I just want to keep the conversation in the open. If you'd like my help with some of these, I'm happy to do that as well. Just give me a shout! Thanks for all the effort - it's going to be a valuable change for our users. |
|
I have made the changes as suggested by you but kept the two lists seperate as users once they create client specific detection points will have that as main source and would not want to have two seperate config for themselves. |
|
@shreyasdn thanks! I'll have a look at this later tonight and review. |
|
@shreyasdn just to clarify ... If the detection point section has configuration for 3 detection points (A1, B1, C1), and the custom detection points section for client application "myapp" has 2 detection points (C2, D2), would you expect the detection points for client application "myapp" to be (A1, B1, C2, D2) or just (C2, D2)? |
|
Merging in ... reviewed - nice work @shreyasdn |
|
@jtmelton sorry about this but had to push a file had made an error in logic. For the earliar question (C2, D2) would be the detection points as they are application specific. |
|
@shreyasdn no problem .. waiting for build to pass. |
|
I agree but on the flip side if we have to give owners access to change the entire config in the UI which may amount to confusion. I feel that an application owner is the right person to adjust the detection points specific to their own needs, but i could be wrong. Maybe we roll this out now and if people want it the other way we change it. The change itself is just one line. Want to hear your thoughts. |
|
I can see it both ways, but I think the most common use case will be that users will want to "inherit" by default the common detection points, and only apply the custom detection points when necessary. Otherwise, I fear we'll get into a situation where we have 30 common detection points, and 5 client applications that each want to override 1 custom detection point each. Instead of 35 detection points getting defined (30 + 5), you'd have 150 (30 * 5). |
|
Thats true let me do this will finish that change and you can merge once and for all. |
|
ok, sounds good |
|
@shreyasdn beautiful, all changes / tests passed ... merging |
For creating client specific detection points
No description provided.