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

Refactor column creation function #1236

Closed
luka-nextcloud opened this issue Jul 24, 2024 · 3 comments · Fixed by #1281
Closed

Refactor column creation function #1236

luka-nextcloud opened this issue Jul 24, 2024 · 3 comments · Fixed by #1281
Assignees
Labels
3. to review Waiting for reviews enhancement New feature or request

Comments

@luka-nextcloud
Copy link
Contributor

Is your feature request related to a problem? Please describe.

Refactor column creation function to avoid passing many arguments

Describe the solution you'd like

No response

Describe alternatives you've considered

No response

Additional context

No response

@luka-nextcloud luka-nextcloud added enhancement New feature or request 1. to develop Accepted and waiting to be taken care of labels Jul 24, 2024
@luka-nextcloud luka-nextcloud self-assigned this Jul 24, 2024
@github-project-automation github-project-automation bot moved this to 🧭 Planning evaluation (don't pick) in 📝 Office team Jul 24, 2024
@luka-nextcloud luka-nextcloud moved this from 🧭 Planning evaluation (don't pick) to 📄 To do (~10 entries) in 📝 Office team Jul 24, 2024
@blizzz
Copy link
Member

blizzz commented Jul 26, 2024

Stems from this comment #944 (review)

First, I agree, there's too many arguments there. Perhaps just reduce this to the very necessary ones, and all optional could be set via dedicated setters on the result object.

But we would have to call all of them anyway, wouldn't we? At least in that case. Otherwise names arguments can be used already (for immediate cure).

Anyways, the methods need an overhaul.

@luka-nextcloud
Copy link
Contributor Author

@juliushaertl @blizzz I can see that function update (https://github.com/nextcloud/tables/blob/main/lib/Service/ColumnService.php#L345) also has many arguments. Should we refactor this function as well?

@juliusknorr
Copy link
Member

Ideally yes, maybe we can come up with a DTO class to contain the column definition that we can pass along instead in other places as well

@luka-nextcloud luka-nextcloud added 3. to review Waiting for reviews and removed 1. to develop Accepted and waiting to be taken care of labels Aug 12, 2024
@github-project-automation github-project-automation bot moved this from 👀 In review to ☑️ Done in 📝 Office team Sep 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews enhancement New feature or request
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants