-
Notifications
You must be signed in to change notification settings - Fork 237
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
Seed search requirements #228
base: master
Are you sure you want to change the base?
Conversation
The initial draft contains a requirements document (my first try), a structure document (my second try) and several notes that still need to be integrated into the requirements. I created the second try, because I thought the first try was too wordy to be useful. Eventually, I wanted to merge both documents to a single one, which contains the contents of both, but I never got around to do so. Also, the last thing I did was to think about the relative center point feature as discussed in this comment, so the current state probably still contains some points that were only useful for this feature. I propose to drop the relative center point feature, since it is too complex. Thus these point can be safely removed. |
Some general remarks:
|
It is useful to center the search around different center points for different search seeds. However, I currently also fail to see why it is useful to have a shape and size, so it might be sufficient to use a center point instead of a search area in the search seed, but to say this for sure, I would have to look closer into it.
You are right. The structure document is nearly a textual domain model, so it is close to the implementation. This is what I tried to say earlier: We should first think about the required functionality and possible conceptual and implementation issues. Afterwards, we can think about an expressive and simple syntax. To give you an example: If the score of |
I've taken a preliminary look at the docs you wrote up @stefandollase. For the most part they look good they just need to be finished. That includes giving some thought to how things will be handled in some cases. My plan at the moment is to go through and work on them this coming Sunday. I think that I will likely set it up as two documents as we really need two things an architecture document and a requirements document. I'll expand and detail seed-search.txt with some tweaks to format for organization. I'll then start rewriting seed-search-structure.txt as an architecture that maps features to classes with implementation notes. I'm inclined to write it as a well noted UML diagram using Modelio to help things be easier to follow visually. I just spent about 15 minutes looking it over and it seems quite up to the task and also open source. |
@stefandollase & @moulins what are your thoughts on doing the architecture as UML with Modelio? |
Yes, that makes sense.
Sounds good :-)
I am unsure about this. I think UML is fine. We should use it to support the explanation of the architecture. However, we should not be dogmatic about it and try to express everything with UML diagrams. Sometimes, a simple explanation is good enough or even more expressive. The point that I am unsure about is the UML tools. I have still to find one that I like. I shortly looked over Modelio and I think we can try it. I don't have the time to evaluate the tool in depth, so I will trust you that it is appropriate for the task. Finally, I think the architecture (currently structure) document is helpful to figure out conceptual as well as some implementation issues. This is why I started to write it. We should try to base the implementation on this document. However, I think that we should not keep it around after the first release of the seed search. This is, because over time we will change and hopefully improve the implementation. This will cause the architecture document to become wrong. Thus, to avoid confusion in the future, my suggestion is to remove the architecture document after the first seed search release is published. This does not mean it is useless: It helps us to get a common idea about the concepts and features. I would like to hear your opinion about this. |
To clarify, I was suggesting that seed-search-structure.txt be moved into the UML doc as comments on classes and sensibly named fields and operations to allow for the data to be visualized. I'm in complete agreement that if we go the UML route, we should not be dogmatic about it. That level of detail is too much for a project like this. My main reason for suggesting it is for visualization. Modelio has a nice feature where you can see the comments in a split view by just clicking on a class or member. That and of all the tools that I tried it seemed to be the most stable with the solid feature set we would need. Essentially, my thoughts were to write the same material as would be in the document using the designer so it had better context and to help think through the code paths as I tend to be a very visual thinker. :-) My opinion is let's write the docs, do the coding and then see what value the document has at that point and make the call then. I'm thinking that it will have enough value to keep, but it is hard to say at this point. If the architecture is so radically changed in a future release or if there is such dogmatically detailed information that it is a maintenance hassle, then it would seem the architecture doc wasn't written well in the first place. My take is that we need to let the code define the methods and fields, the architecture define the classes and feature locations, and the specs define the features (including wishlist ones). If we do that, and we do a tolerable job of writing all three, I think we should be fine keeping all three around. |
@jmessenger235 I don't know your plans, but please don't commit the tool itself to the repository. |
I won't commit the tool. I might commit some brief instructions to make viewing UML easier using the tool for the user, but I wouldn't commit the tool itself. |
So, I've commited my attempt for a document describing how the search will work.
For the presentation, I've made a markdown file, but if you prefer I can convert it to the same style as the other docs. |
@moulins
Non of this involves creating a competing requirements document, nor does it involve creating yet another implementation that does something better or at least different than the current implementation. I am not saying that it is a bad idea to play with the code to get a better feeling for the remaining problems, but the resulting prototype should simply be scrapped. It served its purpose by giving you more insight into the problems and their solutions. However, it does not help us to agree on anything. Pushing for an implementation works fine when working alone on a project/feature. However, this feature is designed by multiple people. I know, it can be harder to follow the thought process of other developers than to simply redo it all. However, that is not an option when working with multiple developers. I am not sure whether you understand this? |
@stefandollase @moulins What's your assessment of where we're at on implementing this? From what it seems like, the next step is a review and completion of the specs that are already here. The markdown files seem like they'll do the job and frankly given that I've not touched this in 2 years they're a much better starting point. |
Is this design finished or does more design work need to be done? I see several requests for this, and several 3rd-party alpha-stage projects which don't do this (yet) or abandoned projects which presumably don't work correctly with 1.14. |
@dhouck @moulins has taken over maintenance of the project. His implementation was the more robust of the two and I know he had done some further work on his branches. What I can say is that the specs in this particular branch never fully flushed out based on the discussion from #120. I'm not sure if they ever got more detailed in any of the other branches or PRs. #186 that got closed may have had some better documentation that could be pulled into this, but I don't believe anyone has invested the time to finalize something that could be used and maintained long term, but I can't say that for sure. |
This pull request should be used to refine the requirements for the seed search feature. It is not supposed to contain any implementation. You can use Githubs feature to comment on differences between files in a pull request to discuss the changes.
To prevent simultaneous changes by multiple people to this branch, I propose the following process:
If you think you reached a useable state, please mention me, but I will also try to keep up with the changes.
Github just introduced pull request reviews, which should improve the proposed workflow. You can use reviews to group up line comments, see here. I suggest to mainly use Comment type reviews for this pull-request.
To directly commit to this pull request, you need to clone the main repository and commit to the branch
seed-search-requirements
.