Skip to content

refactor: filter created entries for report #124

Merged
niemeyer merged 22 commits intocanonical:mainfrom
letFunny:chisel-db-report-filter
Apr 1, 2024
Merged

refactor: filter created entries for report #124
niemeyer merged 22 commits intocanonical:mainfrom
letFunny:chisel-db-report-filter

Conversation

@letFunny
Copy link
Collaborator

@letFunny letFunny commented Mar 5, 2024

  • Have you signed the CLA?

The entries not mentioned explicitly in the slice definition will not be
added to the report (e.g. copyright, parent directories). Additionally,
we have removed the Globbed option and simplified it.

This PR depends on #113.

@letFunny letFunny requested a review from rebornplusplus March 5, 2024 10:09
@letFunny letFunny force-pushed the chisel-db-report-filter branch from 2e39b33 to b2d22d9 Compare March 6, 2024 10:51
Copy link

@rebornplusplus rebornplusplus left a comment

Choose a reason for hiding this comment

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

I left a few comments below. But overall, it looks okay to me.

Note that I only reviewed 622e346 and b2d22d9 as those are the new ones compared to #113.

Path: o.Path,
Mode: o.Mode,
Hash: hex.EncodeToString(rp.h.Sum(nil)),
Hash: hash,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is just a cosmetic change so the hash of folders is an empty string.

"/dir/text-file-3": "file 0644 5b41362b {test-package_myslice}",
},
}, {
summary: "Copyright is installed",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I had to remove the copyright and move it to a separate test because testing several packages was getting too complex and magical.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good, thanks for doing that.

@letFunny letFunny requested a review from niemeyer March 8, 2024 09:31
Copy link
Contributor

@niemeyer niemeyer left a comment

Choose a reason for hiding this comment

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

This is looking great, Alberto, thank you.

A few minor details only.

"/dir/text-file-3": "file 0644 5b41362b {test-package_myslice}",
},
}, {
summary: "Copyright is installed",
Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good, thanks for doing that.

@letFunny letFunny mentioned this pull request Mar 12, 2024
1 task
@cjdcordeiro cjdcordeiro requested a review from niemeyer March 14, 2024 10:05
@cjdcordeiro cjdcordeiro added the Priority Look at me first label Mar 14, 2024
Copy link
Contributor

@niemeyer niemeyer left a comment

Choose a reason for hiding this comment

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

Thanks again for the changes.

@niemeyer niemeyer merged commit 39ba448 into canonical:main Apr 1, 2024
@niemeyer niemeyer changed the title feat: filter created entries for report refactor: filter created entries for report Apr 1, 2024
@letFunny letFunny deleted the chisel-db-report-filter branch October 17, 2024 08:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Priority Look at me first

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants