Skip to content
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

introduce OwnerType #2

Conversation

wxiaoguang
Copy link
Collaborator

No description provided.

Comment on lines +116 to +119
concept_system_global = Global
concept_person_individual = Individual
concept_code_repository = Repository
concept_user_organization = Organization
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

individual and organization are both user types. So maybe concept_user_individual is better?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, feel free to improve~~

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your kindness.
I will make the change later.

@@ -0,0 +1,26 @@
package types
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

About the file path and the package name.
We have many XxxType in different files now, if we add OwnerType into types package in this PR, does this mean we should move the others into this folder/package in the future?

Copy link
Collaborator Author

@wxiaoguang wxiaoguang May 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so, if a type is shared by different packages.

But if a type is specialized for one model, then keep it in its own package.

Copy link
Owner

@yp05327 yp05327 May 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. How about convert the folder name into shared to make it more clearly to other maintainers?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe sharedtypes? I guess golang doesn't like underline in package name ...

Copy link
Owner

@yp05327 yp05327 May 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I edited my comment.
Maybe models/shared/ownertype.go and package name types is good.
We can add more shared things into this folder if we have.
If there are too many shared types, we can move them into models/shared/types/xxx.go in the future.

@yp05327 yp05327 merged commit 1a71427 into yp05327:improve-runer-type-display May 11, 2023
@wxiaoguang wxiaoguang deleted the improve-runer-type-display branch May 11, 2023 04:41
yp05327 pushed a commit that referenced this pull request Apr 8, 2024
Result of `go get -u golang.org/x/net; make tidy`.

This is related to the following vulncheck warning:
```
There are 2 vulnerabilities in modules that you require that are
neither imported nor called. You may not need to take any action.
See https://pkg.go.dev/golang.org/x/vuln/cmd/govulncheck for details.

Vulnerability #1: GO-2024-2687
    HTTP/2 CONTINUATION flood in net/http
  More info: https://pkg.go.dev/vuln/GO-2024-2687
  Module: golang.org/x/net
    Found in: golang.org/x/[email protected]
    Fixed in: golang.org/x/[email protected]

Vulnerability #2: GO-2022-0470
    No access control in github.com/blevesearch/bleve and bleve/v2
  More info: https://pkg.go.dev/vuln/GO-2022-0470
  Module: github.com/blevesearch/bleve/v2
    Found in: github.com/blevesearch/bleve/[email protected]
    Fixed in: N/A
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants