Magento Architectural Discussions, 13 Mar 2019

Previous Meeting Next Meeting

⏯

youtube image

►

From YouTube: Architectural Discussions - March 13th, 2019

Description

Coding Standards.

https://github.com/magento/architecture/issues/77

A

Okay, so that's its architectural discussions, our regular weekly meeting, so we have Alana and the cold right. So we have couple topics from a learn about Korean standards when I can start them.

B

Do you see my screen.

A

B

There so I'll start with the first one is national conflict, so it looks like it's want to keep backward compatibility with all the wedding standards and keep all their under a magenta baptists static. We need to the name of a new coding standard, so I don't know if it matters. What's the name will be R or maybe we can totally replace it. So I need some sites regarded regarding this. Do we actually need to keep all true set.

A

Yes, we haven't the magenta people here and color it now, so I would say that some people may rely on it and yeah.

B

A

B

An example, so, in this case we want to add this, we need to rename.

B

B

Wish, actually, we can just replace it, but it looks like we cannot do.

A

I would remind me to think it should be okay, well,.

B

Then two identical.

C

But isn't it would be backwards incompatible as well renaming the rules? Name is the standard. If someone.

B

C

In their own configuration, it's.

B

Not released yet it's just the develop branch, no.

C

I mean legacy one. We are talking about.

B

A

One will be magenta, that's why we need different name for the new one, so you can reference it.

D

B

So I assume I can.

A

For me, not very important, like both options, that I proposed are fine.

B

So right now for static checks. Vegas HP must attack each person what's new for my son okay, so we cannot deeply invite the court which give us detector. There was both Portland's reflection it it's for me, it's fall, so it doesn't good enough to make some checks.

B

My proposal is to earth pitch based on this tool, uses abstract syntax tree analyze. Php code under the project uses each p-parcel I added few examples. It checks for class existence, existence of property of smaller properties, some chicks like number of passed arguments, for example, its function required.

B

And so on, also, we can take into account that we should extend all those different levels of checks.

B

7 I think this approach will also help us to get this tool with incremental approach. We can start with 0 and then once we fix all of the issues we can move forward. Finally,.

B

Here is the list of just few examples of finders from a gentle boil. I run this tool against this folder, for example, but it has said when functioning worked with two phenomenal sense that.

B

When some classes not found the correct, IP ins and so on, so on also for the long-term plan and future of this tool, it will be good to replace it with to remove which be under support. Instead, it can cover all of the keys.

B

Here was some timing, so it's much more efficient.

B

His perspective is very soon. Example: it took 8 minutes and the person was like 2 hours so my proposal, he said this tool to record their section and you can add this to study tense. That's actually. It I had a few.

E

Have you ever considered Visual Studio code as an ID for developing in PHP is becoming more unpopular.

B

E

We, if we ever consider Visual Studio code as an IDE of preference for developing PHP I, know we're supporting a lot phpstorm, but.

A

So is it related to the static check, or why did you ask.

E

Yes, probably I presume that we have to provide some different sets of tools or.

A

Ok, I was just starting to looking at it, but it's very different for me. I.

E

A

E

Has become an idea of preference, because it's free and phpstorm is not.

A

E

I see you in JavaScript, they use Visual Studio code and it's fully supported on PHP as well, by the way mm-hmm.

A

To look at it, but still for now, I think to make sense to proceed with speech based on. Yes, of course, it looks like a great tool.

A

Now one question so you mentioned are that you need to run the I compile, so we had generated code.

A

Can it be just auto-generated as the classes are requested as well? Did you get the question so I mean? Can you not even run the I compile and yeah, but in this case you.

B

F

Yeah, maybe for this reason we can use a little bootstrap I post it in the comments at a proposal. I think the New Zealand guy has a solution for that. There is a little bit proposal or a little boot strap adaptor for PHP stand that allows to check the code without a DI kanpai, but I think we must check it. Maybe it this is a solution. Then we don't trust around PID I come here. Magento di compile.

A

Not a big deal but I'm just thinking about what will be the best of experience here.

B

We can define our own sets of rules, so it's not necessary. Just to this level, one we can define like those were going to check.

B

What for and the bosses.

A

Okay, so any other concerns, or just we just move on with it.

A

Like no concerns.

A

Hey thanks Adam.

A

Let's see, say: I think you didn't have any other topics, as we still have some time. I wanted to go. My dip through sample requests that we have.

A

A

Let's see okay, so pissed on. There is nothing more to do. I guess we can just merge it right. Then they don't have any plans to do anything else. When it's right.

A

G

Do you know if PHP Stan uses the official PHP winter internally.

B

B

G

Just reading the vh code, Doc's online thing PHP files, and it said the villa's code uses the official PHP linter for PHP language Diagnostics, which allows vias code to stay current with PHP linter improvements.

B

B

G

Just started looking at it, so I'm just wondering if you knew the.

E

Official PHP linter talked about there's, not the HP stand right, you're.

G

Just talking about PHP hell, yeah.

B

But it's those out imports with BT daily.

G

Looks like there's also a PHP stand plugin for obs code for what it's worth.

A

I, don't know phobias code, I, don't know if anybody wants to look into. It would be right.

A

A

What else we have that's a new one.

A

A

Know with composite dependencies for modules, extensions, ok, so I don't know if anybody was looking at it.

A

So that's guessing in the scope of a source, isolation.

A

Well, we can break down the current structure of the module, as you want to have this kind of structure, but.

A

And do not import existing extensions or at least to understand what the impact will be if you're interested in in what is source isolation, you can also join our channel and see what we are doing there. I guess region will present it later when it's ready.

A

Ready so we're looking, how we'll be tracking all the libraries that we use that's over at the meantime.

A

My graphical quality governments, instantiation wonderful I, couldn't have by the way in the meeting yeah.

A

Now this one is maybe also interesting here looking to service isolation, so Eugene was writing down pros and cons of different candidates for the comfort, the composition, also in a stress isolation that he looked Muslim payments, and then we will be looking several other ones, shape, English list, review, stocks, promotions, inventor and pricing and just writing down different complications of each approach like present calls I'm, not Irish approach of each component.

A

That can be separated first for us, so you want to find something that will be not too hard to the compose, but that will still bring some volume.

A

A

Anybody else has any topics, or maybe there is somebody, some authors of any proposals that you want to discuss.

A

It, yes, sorry.

F

I forgot: to put there it's a current issue and coding stomach repository I will open and share my screen.

F

G

F

It's already an pull request and.

F

You shouldn't see my screen now.

E

F

I open a pull request at a match test: first for a sniffs and getter and setter, and currently we have the problem. The current check find.

F

Get us they use a dashing. A typical example of caching can be like like this one. We have very expansive service like a size service.

F

They calculate the size by many parameters on on the fly and asking a web service hey what is through correct size, and for this reason it makes sense to have a caching I could written or caching for size, because if you ask in your application, give me the size three or four times and each time you call it web service, it's very expensive, but in this case anton mentioned, we shouldn't have not or a method they set the property size, but I think it will be profound typical service facade pattern, because in my example, here we have a we have a size service.

F

We have some implementation there. We have a service interface for that and we have the caching for size there. But if we set the size before in some point in our application and the size service or deciced service facet is a static object for application, then to check the found size is set by two two methods and for this example, it can be a false positive or we we don't allow this pattern and I propose to two possible solution. First, we can check if there, if block around the assignment, then it's okay for cashing reason.

F

Also, we can ignore set methods, they start with set or the constructor method and check if there is any order method they that the service Desai's property for for this class, but I, think we should discuss this in the team, because I think it's really known pattern and we using this pattern very often if we have complex servers behind our application.

F

So what do you think about which which of the ways we should go or can somebody take a look on this which examples we should cover or what is maybe the best way to implement this check for the coding star-lord, because I think we have different education there I.

B

Think it's two.

G

Possible solutions, but they.

B

Can be one the first one and the second one, it's a suggestion against I, don't see how.

C

B

If condition like control, structure and also analyzed set, then construct set.

B

Once it's done.

D

Okay, sorry, looking for this court I think you need to use different design for this classes in and you don't need to use, set size method at all, because, as I see when you set size, you don't you won't call the service anymore, because you already set the size so probably need to use some factory to create the this size, how it's called size phosphate with needed size, and you probably don't need to use that size. Public method yeah that maybe this is more question about different design usage, not like static checks.

F

Yeah, but if we have example like this in our code base, it will find it but I think it's yeah I know of what you mean it's more or it's better implementation, if you have a factory for this and set the size over over the factory- and maybe this is also a wrong implementation, but it's very often used on a PHP community I see many example of okay.

F

We have a service behind us and yeah, but okay, then I will implement the first ring, checking for if block or the second and then we'll run again over the co-pays. What do you mean I.

B

F

B

Magenta codebase.

B

F

But which one the first one or the second one sorry I.

B

Think we can combine.

F

A

We once again, so you say the first first one is to check for if blocks and if there is an assignment, it's okay.

A

So so what we're trying to solve like complex, get our sorrow and satyr so yeah.

D

B

Which says that it should not change the stage.

A

Yes, but then what does it mean that if there is assignment, then it is okay? So what is not? Okay,.

A

D

Should not change observable state? We should probably change the.

A

D

Technical guideline yeah.

A

That's what we were discussing so like in this example I mean if you get size returns the same thing always then it's fine, but because it can change. It's probably not really fine, but I, don't know if this rule can and if it's a simple rule to implement right over the date. Such problem.

A

So what we want to all the data's I understand is that if you call getter the visible or observable mentioned state does not change. So if you call gather them, you call another matter method or call getter again, nothing, changes, correct, and in this case this, if is fine the outside, it doesn't change, so you will never get size which is equal to now. You never see it so so yeah I'm trying to understand you mentioned that. If there is an if and assignment, then it is a case what will be filing the test?

A

You know, in which case the testing file.

F

If there is not a if around size like, if you have this line in in this line, not wrapped with, if you have a direct assignment in a gather, more.

A

F

If you have a call or a method and change, it is or our static property. The current check is more like it's more like which I check in for Datak properties. If, if the state has changed over over a method or is a private or public changed by a gap method, the current implementation ask: ok, is this method, a gap method? Ok then run the task for ok. Is there any property change of the class? This is the current implementation, but this implementation far also methods like this one mm-hmm.

D

F

What Antone mentioned this can be fine, it depends on the use case.

A

Try to run to run this check on the code base and see what we have understand. Yes,.

F

I will implement first, if, if they're a wrapped, if statement around the property, then I will ignore and skip, and then we run over the co-pays, and maybe we must also add, constructor and set for this check, but yeah.

F

Then we should think about what is really the use case off of this check, or we will find anything.

F

Okay, thank you.

A

And any other topics.

A

So I'm looking so what su v--'s our, let me show you the screen. What is the proposal that you are discussing think on the last meeting or a couple meetings before this one.

F

Currently, I don't integrate this or make a print of this, but it's I think it shouldn't be really fast to have a brunch with Trump and the basic configuration and what terrorist missingness are some numbers of. How will it cost?

F

It goes over the co-pays, but I think it's yeah I can create a Perot dress at the main repository of Magento, more news from a PHP, and then we can trying to start Crump in this branch and make some features of this branch, but yeah I think we should start with a branch of the current 2.3 develop and integrate their Crump and bright run, some some sample, implementations or but the main problem.

F

What I think is if we, if we create a protest there and some other guys will start implementing stuff on crumb, then we get some point. We must merge. Krump I think it's, maybe not the best way to test the integration, but I don't know what is the better way to do. Read currently.

F

Because you know at some composure dependencies also, it adds a configuration file and if you check out the branch depending on mine branch or on the crumb peach peopre a branch, then we get a dependency in the core. But we don't verify the dependency before and the people must remove krump from their branch before the branch can be merged in Moscow or in the current development branch. It can be a little bit slow development, speed of some people. They testing krump.

A

Mm-Hmm, so once again you mean that if somebody will have this I will have their branch without grant and then we'll will try to merge your branch with Grant. Then they have problems. No.

F

No, no, what what I mean is if the branch is depending on on the grumpy HP branch mm-hm.

A

F

It adds the composer dependency, also the from PHP file. Then we must merge or remove to commit from adding krump from their branch before we can merge the changes to to the development branch. This is only.

A

F

A

Talk about extension developers or the size, for example, already use ground, PHP right, I know.

F

What I mean, as we discussed in the last meeting, we have some coop people, they may be try to starting development with Trump, and then we add some numbers or what I have more numbers, how it it will be, go for the team or it slowdowns the development speed, because if I add some numbers, then we have yeah like okay, we, we are creating an issue and developing and we have some not really trustful numbers.

F

What you? What I mean is we get some testing numbers but not real numbers? And maybe, if we have later problems with real numbers or real progress, then we already integrate krump. This is only one what one thing: what I have in the mind: mm-hmm I.

A

Don't know, let's still try to use it. It's just probably important to explain to those people who will be trying to use it now that, yes,.

F

A

What they all need to do, but yeah I, think it's just details of this transition period.

F

Yeah we can, we can add us and I I. Think I should add some documentation there to the to the protest. How how can people start using Trump for their prayer requests and before are we merging this pull request? We must only remove one commit off of the branch and hopefully it doesn't ends and marching how if we are merging Trump, but it shouldn't be, it shouldn't be fine. I.

A

A

What do you agree on we'll be testing it? Maybe I'll test it without committing it or I, don't know yeah. So I would like to have it because, as soon as we committed it, it will go to entire develop to all developers to everybody. So we need to test it on a dedicated branch or maybe not commit as not committed code or something like that and test and see what are the results and only one. We are pretty sure we can merge it. Yeah.

F

Yeah that sounds good. Okay, next step from years have a basic integration of krump in 2.3, develop this krump all we have 2.3 developed, krump s branch and then I write the documentation how people can use krump for for their prove request, and then we get more more numbers. How how we solve the problem of the core team, or it's useful or mm-hmm.

A

Okay, good and I guess we can merge this proposal. Can you sign si la vie? You just add it's a lie. Hang it to the sleeper oh yeah.

F

No promise it's no problem. I I. Do it.

A

Okay, don't think I have anything else unless any other topic appeared then see you next week, thanks for joining by.

F