Diagnostics icon indicating copy to clipboard operation
Diagnostics copied to clipboard

directory modelling suggestion

Open entunidotdeb opened this issue 3 years ago • 7 comments

proposing something like this (or better & complete) to have all properties of directory like includeHiddenFiles, maxDepth etc. ik this is quite a big change and would not be backward compatible but i guess worth to include something similar. your thoughts on this? @AvdLee

entunidotdeb avatar Aug 13 '22 17:08 entunidotdeb

Messages
:book:

View more details on Bitrise

:book: DiagnosticsTests: Executed 33 tests (0 failed, 0 retried, 0 skipped) in 0.482 seconds

SwiftLint found issues

Severity File Reason
Warning DirectoryTreeReporter.swift:18 Lines should not have trailing whitespace. (trailing_whitespace)
Warning DirectoryTreeReporter.swift:78 Lines should not have trailing whitespace. (trailing_whitespace)

Code Coverage Report

Name Coverage
Diagnostics 70.42% ⚠️

Generated by :no_entry_sign: Danger Swift against 105b10469e4d727460c5df9b9588ea3b09ace26b

wetransferplatform avatar Aug 14 '22 12:08 wetransferplatform

Thanks for this, @entunidotdeb! Can you provide a bit more rationale for this change?

to have all properties of directory like includeHiddenFiles, maxDepth etc

What makes this an improvement over the current API, for example?

Also given that this would break the API, it'd technically require a major version bump, so that's at least something to keep in mind.

BasThomas avatar Aug 15 '22 06:08 BasThomas

@BasThomas so currently for a client to have multiple doc reports of different maxDepth, includeHiddenFiles and other properties, client would have to create multiple instances of DirectoryTreesReporter. Even then client would have no option to specifically set maxDepth for a group out of the nested Group/Directory Tree. How about client has option to customize the Directory specific props instead of the DirectoryTreesReporter props ?

multiple DirectoryTreesReporter would lead to multiple DiagnosticsChapter

entunidotdeb avatar Aug 15 '22 07:08 entunidotdeb

Gotcha, thanks!

BasThomas avatar Aug 15 '22 07:08 BasThomas

@AvdLee @BasThomas I have updated this PR with new init for DirectoryTreeReporter and deprecating the earlier init. you may review and suggest changes if needed.

entunidotdeb avatar Aug 15 '22 22:08 entunidotdeb

@entunidotdeb the code looks great, but it seems that tests are failing. Did you manage to run tests successfully locally?

AvdLee avatar Sep 15 '22 09:09 AvdLee

@entunidotdeb the code looks great, but it seems that tests are failing. Did you manage to run tests successfully locally?

@AvdLee yes sir

entunidotdeb avatar Sep 17 '22 09:09 entunidotdeb

This PR is stale because it has been open for 30 days with no activity. Remove the Stale label or comment or this will be closed in 10 days.

github-actions[bot] avatar Oct 20 '22 00:10 github-actions[bot]

@entunidotdeb our repository requires signed commits. Would it be possible for you to enable signed commits and recommit using the following command:

 git rebase --exec 'git commit --amend --no-edit -n -S' -i develop
 git push --force

You can read more about setting up signed commits here.

AvdLee avatar Oct 24 '22 11:10 AvdLee

@entunidotdeb we're almost there! Your commits are signed but unverified:

CleanShot 2022-10-31 at 09 50 58@2x

You can learn more about that here: https://docs.github.com/en/authentication/managing-commit-signature-verification/displaying-verification-statuses-for-all-of-your-commits

AvdLee avatar Oct 31 '22 08:10 AvdLee

@AvdLee Done

entunidotdeb avatar Oct 31 '22 10:10 entunidotdeb

Thanks for going over the comments @entunidotdeb! 🚀

entunidotdeb avatar Nov 01 '22 15:11 entunidotdeb

@entunidotdeb congrats with your successful contribution! Thanks again for setting up signed commits 🙏

AvdLee avatar Nov 03 '22 10:11 AvdLee

Thank you @AvdLee for suggestions on PR, approval and merge. Thanks @peagasilva for approval.

entunidotdeb avatar Nov 03 '22 11:11 entunidotdeb

Congratulations! :tada: This was released as part of Release 4.3.1 :rocket:

Generated by GitBuddy

wetransferplatform avatar Dec 05 '22 11:12 wetransferplatform