-
Notifications
You must be signed in to change notification settings - Fork 6
add ”LLPkg Details“ meta info for llpkg #21
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
base: goplus
Are you sure you want to change the base?
Conversation
…into feat/llpkg_route
feat: add "llpkg" route, migrate all gomodule headers containing the "LLPkg Library" that include "github.com/goplus/llpkg".
|
/cc @luoliwoshang |
internal/postgres/llpkg.go
Outdated
| if llpkg.IsOfficialLLPkgModule(modulePath) { | ||
| return nil, fmt.Errorf("%s@%s is not a LLPkg module", modulePath, version) | ||
| } |
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.
When llpkg.IsOfficialLLPkgModule(modulePath) returns true, logically, this should mean that modulePath is an official LLPkg module. However, the code block returns an error, with the error message indicating that modulePath is not an LLPkg module.This should be considered a logical error.
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.
done at da2d8b3
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.
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.
We currently only consider official LLPkgs.
| } | ||
|
|
||
| // GetLLPkgConfig is not supported at FetchDataSource is will always return an error. | ||
| // It should be implemented in the postgres package. |
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.
maybe determine this situation more clearly
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.
it is mentioned at fetchdatasource package's top
// Package fetchdatasource provides an internal.DataSource implementation
// that fetches modules (rather than reading them from a database).
// Search and other tabs are not supported.
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.
I don't mean to comment on the function of FetchDataSource, but to comment on why GetLLPkgConfig is not implemented in FetchDataSource
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.
Because there is no definite answer at the moment, the last update comment is TODO at 23655bb
feat: better error resolve
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.
For yours question about a submodule without license can't show document in pkgsite.
I Used github.com/NEKO-CwC/llpkgstore as a GithubRepo for testing, and tested it with the included cjson. It appears that the module directory where cjson is located (i.e., the cjson directory itself) does not have a license, so its actual documentation information cannot be displayed.
same licence lack info in pkg.go.dev https://pkg.go.dev/github.com/NEKO-CwC/llpkgstore/cjson
but in this repo https://pkg.go.dev/github.com/luoliwoshang/goplus-llpkg/libxslt,it have not direct licence in the libxslt path,but it can show document.
but in the github.com/luoliwoshang/goplus-llpkg/libxslt , it have the release version, https://pkg.go.dev/github.com/luoliwoshang/goplus-llpkg/libxslt?tab=licenses
@NEKO-CwC may release your test cjson for test
but in github.com/luoliwoshang/goplus-llpkg/cjson it havent direct licence & it havent a release,it also can show its document.

❯ curl -H "Accept: application/vnd.github.v3+json" https://api.github.com/repos/luoliwoshang/goplus-llpkg/releases | jq '.[].tag_name'
% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed
100 56341 0 56341 0 0 68690 0 --:--:-- --:--:-- --:--:-- 68624
"libxml2/v1.2.1"
"libxslt/v1.0.1"
"libxslt/v1.0.0"
"libxml2/v1.2.0"
"zlib/v1.0.0"
"libxml2/v1.1.0"
"libxml2/v1.0.0"
(base) 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.
After researching this issue, I can explain why the "Redistributable license False" error occurred and why Documentation wasn't displaying properly.
Go package behavior with goproxy
- When packaging, Go automatically inherits LICENSE from the root directory (if present)

- When fetching from goproxy:
- Latest release is pulled first (even with newer tags available)
- Without releases, latest tag is used
- Without tags, latest commit is used to create a pseudo-version
Root cause analysis
The issue occurred because when fetching cjson/v1.1.0 tag, this version didn't include a LICENSE in the root directory yet. The LICENSE was added in commit 36ed437, which came immediately after cjson/v1.1.0. Since the package zip pulled from goproxy didn't contain a LICENSE, pkgsite displayed "Redistributable license False".
Test case explanation
github.com/luoliwoshang/goplus-llpkg/libxsltworks because it has a release with LICENSE in rootgithub.com/luoliwoshang/goplus-llpkg/cjsonworks in your test environment because it uses the latest commit's pseudo-version, which includes LICENSE
Resolution
This has been fixed in cjson/v1.4.0, which properly displays Documentation now that LICENSE is included.
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👍
| return results, nil | ||
| } | ||
|
|
||
| // TODO: It is temporarily not implemented. |
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.
A Note is recommended to use "MARKER(uid): note body" format.
Details
lint 解释
这个lint结果提示你建议使用“MARKER(uid): note body”格式来编写注释。这意味着在代码中添加注释时,应该遵循这种特定的格式,以便于识别和理解。
错误用法
以下是一个错误的注释示例:
// 这是一个错误的注释示例正确用法
以下是一个正确的注释示例:
// MARKER(12345): 这是一个正确的注释示例💡 以上内容由 AI 辅助生成,如有疑问欢迎反馈交流
|
This PR needs corresponding tests, e.g., checking if nested directories display under \LLPkg and if an LLPkg shows the expected LLPkgDetail. |
|
Current Pkgsite's development uses lots of path replacements, such as substituting the |
feat: logic change in llpkg.cfg verify
|
[Git-flow] Hi @NEKO-CwC, There are some suggestions for your information: Rebase suggestions
Which seems insignificant, recommend to use For other DetailsIf you have any questions about this comment, feel free to raise an issue here: |


Add the "llpkg_config" column to the "modules" table in the database, save the "llpkg.cfg" file content in it
Add


LLPkg Detailsline of meta information to the unit page to display the llpkg info.Add a new label
LLPkg libraryat headerAdd a new environment variable "LLPKG_METADATA_DIR" for storing the llpkgstore.json through llpkgstore/metadata.
Add a new path "/llpkg", which behaves and displays content similar to the official "/std", and constructs all LLPkgs for display through the hosted llpkgstore.json file.
