Rename Resolver types to include 'Resolver'#69926
Merged
oatkiller merged 1 commit intoelastic:masterfrom Jun 25, 2020
Merged
Conversation
oatkiller
added
Feature:Endpoint
Contributor
|
Pinging @elastic/endpoint-app-team (Feature:Endpoint) |
bkimmel
reviewed
Jun 25, 2020
Contributor
There was a problem hiding this comment.
❔ I've been trying to stop saying "Resolver" as much in favor of "GTV", I know they are changing the name for this officially, so it might be worth asking when we stop calling it that in our code? I'm not suggesting "right here, right now" just want to flag the issue.
Contributor
Author
There was a problem hiding this comment.
Few thoughts here:
- I think that naming a project too early can be dangerous. If the scope and definition of a project is complete (won't change ever again), then naming it in a way that describes what it does is fine. But if you rename something like Resolver to 'Graphical Timeline', then people start thinking 'oh we should do A because this is a graphical timeline', or 'we shouldn't do b, because that doesn't fit the description of a graphical timeline.' Resolver isn't mature and its definition is going to be changing every release.
- I'm hoping that this project isn't always specific to Security Solution. 'Graphical Timeline' is a branding term for a specific feature in Security Solution. That feature uses Resolver, but Resolver might be used elsewhere for other things. In other words, I'm thinking of Resolver as the mapping/graphing technology.
- If the term Graphical Timeline sticks around long enough that there becomes confusion, then we should reconsider the name of the project. I'm in a meeting right now and I've heard 'Resolver' said many times.
- Graphical Timeline is an awkward name for a code project. Having a generic and vague name might be good for branding, but it would probably be confusing in our code base. For one thing, Resolver is entirely unrelated to Timeline, which is also a project in Security Solution.
jonathan-buttner
approved these changes
Jun 25, 2020
Contributor
jonathan-buttner
left a comment
jonathan-buttner
left a comment
There was a problem hiding this comment.
Changing the names is fine with me. We'll need to update the api_integration tests as well:
Contributor
Author
I'm curious why this isn't failing typecheck (locally at least.) I'm going to wait and see what happens on CI. |
Contributor
Looks like it failed in CI: https://github.com/elastic/kibana/pull/69926/checks?check_run_id=807607380 That is odd that it didn't fail locally 🤔 I wonder if the |
Include the word 'Resolver' in some Resolver specific types in order to improve readability and ease of auto-importing.
oatkiller
force-pushed
the
renaming-resolver-types
branch
from
d781b0c to
56309a0
Compare
Contributor
Author
yah, i just goofed lol |
Contributor
💚 Build SucceededBuild metricspage load asset sizebeta
History
To update your PR or re-run it, just comment with: |
oatkiller
pushed a commit
to oatkiller/kibana
that referenced
this pull request
Jun 25, 2020
Include the word 'Resolver' in some Resolver specific types in order to improve readability and ease of auto-importing.
oatkiller
pushed a commit
that referenced
this pull request
Jun 25, 2020
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Include the word 'Resolver' in some Resolver specific types in order to improve readability and ease of auto-importing.
I found that this made it easier for me to read and understand some code while working on #69728
Checklist
Delete any items that are not applicable to this PR.
For maintainers