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

feat: Add current folder context for file list actions #1113

Merged
merged 2 commits into from
Nov 13, 2024

Conversation

Pytal
Copy link
Contributor

@Pytal Pytal commented Nov 8, 2024

Signed-off-by: Christopher Ng <chrng8@gmail.com>
@Pytal Pytal added enhancement New feature or request 3. to review labels Nov 8, 2024
@Pytal Pytal requested review from susnux and skjnldsv November 8, 2024 19:23
@Pytal Pytal self-assigned this Nov 8, 2024
Copy link

codecov bot commented Nov 8, 2024

Bundle Report

Bundle size has no change ✅

Signed-off-by: Christopher Ng <chrng8@gmail.com>
Copy link
Contributor

@skjnldsv skjnldsv left a comment

Choose a reason for hiding this comment

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

While I do get the point of object parameters, mixing both seems weird. I would pass the directory directly, not within an object.
We do the same in the new file menu

@skjnldsv
Copy link
Contributor

Next release is a minor, let's wait for this PR to be in before releasing the next minor?

@susnux
Copy link
Contributor

susnux commented Nov 13, 2024

Next release is a minor, let's wait for this PR to be in before releasing the next minor?

No 3.10.0 was never released so we can put it into 3.10.0

@Pytal Pytal merged commit 7a25265 into main Nov 13, 2024
17 checks passed
@Pytal Pytal deleted the feat/current-folder-context branch November 13, 2024 12:28
@susnux
Copy link
Contributor

susnux commented Nov 13, 2024

Damn did not saw the auto merge

@susnux
Copy link
Contributor

susnux commented Nov 13, 2024

While I do get the point of object parameters, mixing both seems weird. I would pass the directory directly, not within an object.

Having more than (I think 2, others say 3) parameters is often a mess, especially when this is only passing options.
So in most cases it makes sense to have:

function doSomething(value, options)

so you do not end up with a lot of parameters.

But on the other hand having many parameters / options means that your function should be refactored as it is doing to many things and probably can be split...

@susnux susnux mentioned this pull request Nov 13, 2024
1 task
@skjnldsv
Copy link
Contributor

Aaaand now it's released... Was a bit fast and ignoring my comment @susnux

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants