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

Refactoring and removing check.v1 #20

Merged
merged 7 commits into from
May 20, 2024
Merged

Conversation

pjbgf
Copy link
Member

@pjbgf pjbgf commented Apr 8, 2024

This PR refactors this library. Summary of changes:

  • Remove dependency to gopkg.in/check.v1 as that project is no longer being maintained.
  • Add support for in-memory use.
  • Create an internal billy.Filesystem that is based on an underlying embed.FS.

Signed-off-by: Paulo Gomes <paulo.gomes@suse.com>
This representation is mostly useful within the context of go-git-fixtures.
go-billy Filesystem interface is too generic, which makes this implementation
violate the Liskov Principle. When more narrow representations of the filesystem
are available in go-billy, this could be moved upstream.

Signed-off-by: Paulo Gomes <paulo.gomes@suse.com>
@pjbgf pjbgf force-pushed the improve-tests branch 2 times, most recently from 8251fe9 to 276b7c3 Compare April 8, 2024 08:35
The refactoring achieves a few goals:
- Support for fully in-memory execution, without the need of extracting
tgz files into disk.
- Removal of the deprecated check.v1 dependency.

Signed-off-by: Paulo Gomes <paulo.gomes@suse.com>
@pjbgf pjbgf changed the title Refactoring and bump to v5 Refactoring and removing check.v1 Apr 8, 2024
Signed-off-by: Paulo Gomes <paulo.gomes@suse.com>
Signed-off-by: Paulo Gomes <paulo.gomes@suse.com>
@pjbgf pjbgf requested a review from hiddeco April 11, 2024 07:48
tmpPrefix = "tmp-tgz-"
)

type Option func(*options)

Choose a reason for hiding this comment

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

What is an Option?

internal/tgz/tgz.go Outdated Show resolved Hide resolved
@aymanbagabas
Copy link

We should probably add lint configs to enforce rules like comments and code quality

@pjbgf
Copy link
Member Author

pjbgf commented Apr 26, 2024

@aymanbagabas added some linters and fixed the initial issues, we can expand the linters list as we go. We should roll them out across the other repositories as well. PTAL

@pjbgf pjbgf requested a review from aymanbagabas April 26, 2024 06:51
Signed-off-by: Paulo Gomes <paulo.gomes@suse.com>
Signed-off-by: Paulo Gomes <paulo.gomes@suse.com>
test:
$(GOTEST) ./...

generate: $(esc)
$(GOCMD) generate
validate: validate-lint validate-dirty ## Run validation checks.

Choose a reason for hiding this comment

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

Is this necessary? Instead, we can use a build workflow to run the linter. Contributors should have their editor linter set up.

Copy link
Member Author

Choose a reason for hiding this comment

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

The idea here is mostly to align CI and local, with the least amount of assumptions about a developer/contributors' environment.

Copy link
Member Author

Choose a reason for hiding this comment

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

Merging it as is, but happy to discuss further what this should look like across the org.

Comment on lines +25 to +27
- name: Validate
if: matrix.platform == 'ubuntu-latest'
run: make validate

Choose a reason for hiding this comment

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

Suggested change
- name: Validate
if: matrix.platform == 'ubuntu-latest'
run: make validate
- name: Validate
if: matrix.platform == 'ubuntu-latest'
run: make validate

@pjbgf pjbgf merged commit 48b0644 into go-git:master May 20, 2024
9 checks passed
@pjbgf pjbgf deleted the improve-tests branch May 20, 2024 13:16
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.

None yet

2 participants