Skip to content

100 days party (+ Storage Module Basis) #34

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

Merged
merged 35 commits into from
Jun 16, 2021
Merged

100 days party (+ Storage Module Basis) #34

merged 35 commits into from
Jun 16, 2021

Conversation

paulotruta
Copy link
Member

Uptime milestones (100 days and 1 year) will be given a little celebration emoji :)
image

PS: Needs prior merge from storage-module, please merge storage-module branch to main first. Then do git merge storage-module --no-ff before merging this one

paulotruta added 29 commits June 3, 2021 09:28
Copy link
Contributor

@inverse inverse left a comment

Choose a reason for hiding this comment

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

Nice work 👍🏼

return [];
}

return json_decode($storage_devices_list_option->getValue(), true);
Copy link
Contributor

Choose a reason for hiding this comment

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

Def thinking the way we're decoding these options into json arrays we should perhaps think about mapping these to real types... not sure what would be best as we could manually do it for the PHP side which would be sufficient but it's also from the go side right pushing them in.. opinions?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, agree about decoding into native types, as we can potentially validate the data even before it gets into the controller.

Validation is happening on the Go side thanks to Marshall and Unmarshall requirement of having to define these json structures as go types.

I also agree that we can use pure PHP for this, and gain a few neat features such as data types and expected values validation, a better structured codebase and thus cleaner code, and way less confusion on the controller side.

My suggestion is that we create the types and a class for serializing and deserializing these types from and to json. Then we replace the current option calls in the codebase with the native types ones which return known structures to PHP. Will be fun 😉

I see a good potential improvement here. Let's open a new issue based on this as the scope of doing it is probably gonna touch more parts of the codebase.

Copy link
Member Author

Choose a reason for hiding this comment

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

Opened issue #37 so we can follow up on this ;)


<div class="">
<div class="" style="margin-top: -25px;">
{# {{ include('partials/_row_title_block.html.twig', {main_title: 'Your Storage Buckets', button_title: 'Create Bucket', button_link: '/storage/bucket/new', new_tab: false})}}
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be commented out?

Copy link
Member Author

@paulotruta paulotruta Jun 16, 2021

Choose a reason for hiding this comment

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

Yes, was just an experiment, but storage buckets will be a whole new feature to be worked in the scope of another PR / Sprint.
Just left it there as I mocked a base UI, can be useful to quickly get going when we get to it.

@paulotruta paulotruta changed the title 100 days party 100 days party (+ Storage Module Basis) Jun 16, 2021
@paulotruta paulotruta merged commit 2b6059c into main Jun 16, 2021
@paulotruta paulotruta deleted the 100-days-party branch June 16, 2021 14:08
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.

2 participants