-
Notifications
You must be signed in to change notification settings - Fork 11
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
Refactor issue, queryissue pkgs #2044
Conversation
@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and | |||
limitations under the License. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need the prefix query
in the directory names of queryissue
, queryparser
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the query directory is just better file organization, it does not translate to any form of nesting of packages. If there was indeed some form of nesting, I agree; it would make sense to remove the "query" prefix. But since these are top-level packages, I wanted to stick to the pattern of keeping the folder name same as the package name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Describe the changes in this pull request
Some more refactoring on top of comments from #1873
issue
pkg intoqueryissue
pkgissue
pkg intoqueryIssue
pkg.IssueInstance
toqueryIssue
queryissue
andqueryparser
under a common folderquery
Describe if there are any user-facing changes
N/A
How was this pull request tested?
N/A. Existing tests suffice.
Does your PR have changes that can cause upgrade issues?