@wojsmol I'm sorry, but i don't understand your workflow...

You apply code style here for some files (php + xml) but you do some and not all...
Why?

Isn't it easier to focus on all php or xml files inside on component, instead of doing some but not all, and mixing php and xml... This does not help to know if one extension is really full reviewed or not...
I would recommend to focus first on all xml (as i did for (first in admin) com_categories, com_newsfeeds, com_contact). Of course only my point of view, but IMO, could be good to know what's done in a PR, and what's to be done...

;-)

That's my fault. I asked for all the remaining fixes to be on one pr after
discussing with some of the committers during jab.
On 23 May 2016 3:11 pm, "Cyril Rezé" notifications@github.com wrote:

@wojsmol https://github.com/wojsmol I'm sorry, but i don't understand
your workflow...

You apply code style here for some files (php + xml) but you do some and
not all...
Why?

Isn't it easier to focus on all php or xml files inside on component,
instead of doing some but not all, and mixing php and xml... This does not
help to know if one extension is really full reviewed or not...
I would recommend to focus first on all xml (as i did for (first in admin)
com_categories, com_newsfeeds, com_contact). Of course only my point of
view, but IMO, could be good to know what's done in a PR, and what's to be
done...

;-)

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#10602 (comment)

Thanks @brianteeman !
So what is the rule there, now ? About code style ?

@wojsmol see you just added other not reviewed files ????
Ping me when done, will review ;-)

The idea is that once you have done a few small ones to establish the style
/ changes and they have been accepted then the rest can be in a single pr.
This will hopefully reduce everyone's workload. If it doesn't work we can
always review the process.

@brianteeman I agree! ????
This is why i asked about not reviewed files in the com_banners, and only some reviewed... (seems to me as not a ready to be reviewed PR...).

So, in case of components and code styles, could be complex to do all components in the same PR.

• Should we split each component code style PR ? (seems more easy to prevent conflict, and help review).
• Should we split php and xml files code style review ?

About this current PR, my issue was mainly to see not all xml files and not all php files reviewed ;-)

@JoomliC Now I corrected all the com_banners files

@wojsmol just started to check config.xml, and see at start a not reviewed field... Could you check your changes before? Thanks!

@JoomliC I'll check as soon as github starts to function normally https://status.github.com/messages

@wojsmol Yes, not working well today ;-) (for me, seems just back ok after 1 hour with no synchronization possible...)

@JoomliC I reviewed all the files again

@wojsmol Just added one note on banners.xml
Everything else seems ok! ????

@JoomliC New line added ????

I have tested this item ✅ successfully on 02064d6

On code review

@wojsmol Thanks!

I have tested this item ✅ successfully on 02064d6

on code review
thanks

RTC