feat: allow custom name by MeteorsLiu · Pull Request #60 · goplus/llpkgstore

MeteorsLiu · 2025-04-25T10:29:58Z

Changes for custom name:

llpkgstore generate retrieve the package name from user-provided directory name for generating llcppg.cfg
llpkgstore verfication retrieve the custom name from parsing the last element of module path in go.mod.

For example,
go.mod:
```
module github.com/goplus/llpkg/libcjson

go 1.22
```
Its custom name is libcjson
CheckPR finds out the real llpkg root directory containing llpkg.cfg / llcppg.cfg / go.mod from all changed files.
The name in llpkg.cfg will be only used in llpkgstore.json.To clarify their difference, ClibName represents the name in llpkg.cfg while Name represent the actual name(custom name).
Release-as: directive must match the custom name, not the clib name, like

directory structure:

-- libcjson
       |-- llpkg.cfg ...

Its Release-as: directive is like Release-as: libcjson/v1.0.0

Test: MeteorsLiu/llpkg#78
https://github.com/MeteorsLiu/llpkg/actions/runs/14690247173

codecov · 2025-04-27T06:46:08Z

Codecov Report

Attention: Patch coverage is 23.45277% with 235 lines in your changes missing coverage. Please review.

Project coverage is 41.95%. Comparing base (237b8f9) to head (f8772e1).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
internal/actions/core.go	0.00%	175 Missing ⚠️
internal/actions/llpkg/package.go	65.51%	17 Missing and 3 partials ⚠️
internal/actions/api.go	0.00%	16 Missing ⚠️
internal/actions/actions.go	40.00%	12 Missing ⚠️
internal/actions/llpkg/type.go	0.00%	8 Missing ⚠️
cmd/llpkgstore/internal/verification.go	0.00%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #60      +/-   ##
==========================================
+ Coverage   39.68%   41.95%   +2.27%     
==========================================
  Files          32       35       +3     
  Lines        1968     2052      +84     
==========================================
+ Hits          781      861      +80     
+ Misses       1089     1083       -6     
- Partials       98      108      +10

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Co-authored-by: 张之阳 <51194195+luoliwoshang@users.noreply.github.com>

qiniu-x · 2025-04-28T03:34:52Z

[Git-flow] Hi @MeteorsLiu, There are some suggestions for your information:

Rebase suggestions

Following commits have duplicated messages

fix: ci

fix: ci

fix: ci

Which seems insignificant, recommend to use git rebase command to reorganize your PR.

For other git-flow instructions, recommend refer to these examples.

Details

If you have any questions about this comment, feel free to raise an issue here:

https://github.com/qiniu/reviewbot

luoliwoshang · 2025-04-28T07:13:46Z

+	t.Run("one-go-file", func(t *testing.T) {
+		tempGoFileName := filepath.Join(demoDir, "x.go")
+		err := os.WriteFile(tempGoFileName, []byte(`package libcjson`), 0644)
+		if err != nil {
+			t.Error(err)
+			return
+		}
+		defer file.RemovePattern(filepath.Join(demoDir, "*.go"))
+
+		checkName(t, demoDir, false)
+	})
+
+	t.Run("multi-go-files", func(t *testing.T) {
+		tempGoFileName1 := filepath.Join(demoDir, "a.go")
+		err := os.WriteFile(tempGoFileName1, []byte(`package libcjson`), 0644)
+		if err != nil {
+			t.Error(err)
+			return
+		}
+		tempGoFileName2 := filepath.Join(demoDir, "x.go")
+		err = os.WriteFile(tempGoFileName2, []byte(`package cjson`), 0644)
+		if err != nil {
+			t.Error(err)
+			return
+		}
+		defer file.RemovePattern(filepath.Join(demoDir, "*.go"))
+
+		checkName(t, demoDir, false)
+	})


table-driven test maybe is also better

Some tests require multiple files, these logics cannot be reused.

Perhaps could use a slice like []struct{name, content string} to hold each file’s name and content, and then simply loop over that slice to write out the files—it should work just fine.

I think in that case, simply use t.Run may be better, should we change to table-driven style?

What needs to be clarified is that t.Run is merely the mechanism for running subtests, whereas table-driven is a testing style—they are not mutually exclusive. In this case, we can embed t.Run within a table-driven setup to avoid repeatedly writing and cleaning up files.

and in this case,we actual have four similar test case,like,multi-go-files-fallback,no-go-files,multi-go-files,one-go-file,it have redunant logic here.

luoliwoshang · 2025-05-07T06:51:00Z

 //	cfg: Package configuration
-func (d *DefaultClient) checkVersion(ver *versions.Versions, cfg config.LLPkgConfig) error {
+func (d *DefaultClient) checkVersion(ver *mappingtable.Versions, pkg *llpkg.LLPkg) error {
 	// 4. Check MappedVersion


some comment like this is deprecated

luoliwoshang · 2025-05-07T07:11:45Z

+type LLPkg struct {
+	cfg config.LLPkgConfig
+
+	packageName string


NIT:indicate where to fetch the packageName to reduce misunderstandings—after all, this update introduces two package names

Suggested change

packageName string

packageName string // package name from go file.

And this field's type consist with PackageName as other struct is better ? the PackageName describe the custom name of llpkg.

Exporting PackageName will make it mutable, which it's not expected. packageName should be only readable after initialization

i mean packageName 's type change string to type PackageName

MeteorsLiu added 3 commits April 25, 2025 18:29

feat: allow custom name

920f756

fix: default to current dir when package path is empty

5258a2b

fix: ci

418f0b8

MeteorsLiu marked this pull request as ready for review April 27, 2025 06:46

MeteorsLiu added 2 commits April 27, 2025 06:50

fix: add test

ceb2311

fix: add nil check

2c56c04

MeteorsLiu requested review from aofei and luoliwoshang April 27, 2025 07:10

MeteorsLiu added 2 commits April 27, 2025 07:57

fix: use dir name instead of llpkg.cfg package name

56d3dea

fix: use user-provided go.mod last element name for package name

3220284

MeteorsLiu marked this pull request as draft April 27, 2025 08:20

fix: install before verifcation

575d87f

MeteorsLiu marked this pull request as ready for review April 27, 2025 08:42

luoliwoshang reviewed Apr 27, 2025

View reviewed changes

Comment thread internal/actions/llpkg/package_test.go Outdated

Comment thread internal/actions/llpkg/package_test.go Outdated

Comment thread internal/actions/api.go

Comment thread internal/actions/api.go

Comment thread internal/actions/api.go Outdated

Comment thread internal/actions/api.go

MeteorsLiu and others added 7 commits April 28, 2025 09:33

Update internal/actions/llpkg/package_test.go

3dd955b

Co-authored-by: 张之阳 <51194195+luoliwoshang@users.noreply.github.com>

feat: add new type to avoid misusing the function

6b13942

merge

ee344d5

fix: remove old version implement

7360a43

fix: ci

19e118f

refactor: parse package name instead

2ea2800

fix: ci

f7f532d

MeteorsLiu added 2 commits April 28, 2025 14:24

fix: add err check for getting workflow run id

c14824f

chore: move exported function to core

0916b16

luoliwoshang reviewed Apr 28, 2025

View reviewed changes

MeteorsLiu added 4 commits April 28, 2025 16:37

fix: use table-driven test

e7e0785

fix: adjust var name

8b29ff3

fix: add comment

d8e5540

chore: adjust some details

f8772e1

luoliwoshang reviewed May 7, 2025

View reviewed changes

Comment thread internal/actions/llpkg/package.go

	packageName string
	packageName string // package name from go file.

Conversation

MeteorsLiu commented Apr 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Apr 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

qiniu-x Bot commented Apr 28, 2025

Rebase suggestions

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MeteorsLiu May 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

MeteorsLiu commented Apr 25, 2025 •

edited

Loading

codecov Bot commented Apr 27, 2025 •

edited

Loading

MeteorsLiu May 7, 2025 •

edited

Loading