Closed Bug 1023816 Opened 11 years ago Closed 9 years ago

Extract <gaia-confirm> from Gaia repo

Categories

(Firefox OS Graveyard :: Gaia, defect)

Product:
Firefox OS Graveyard
Component:
Gaia
Platform:
x86
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED INVALID

People

(Reporter: wilsonpage, Unassigned)

Details

Attachments

(1 file)

pull-request (master)
46 bytes, text/x-github-pull-request
kgrandon
: feedback+
vingtetun
: feedback+
Details | Review
No description provided.
Attached file pull-request (master) —
Attachment #8438402 - Flags: review?(kgrandon)
gaia-confirm has been pulled out pulled out of Gaia into its own repo [1]. I have ported the two apps that were using it ('verticalhome' and 'collections'?), and deleted `/shared/elements/gaia_confirm/`. [1] https://github.com/gaia-components/gaia-confirm
Comment on attachment 8438402 [details] [review] pull-request (master) I am going to hold off on R+, but leave F+ for now until I have a better picture of how everything works together, and we get sign-off from the necessary people.
Attachment #8438402 - Flags: review?(kgrandon) → feedback+
Wilson - One of my main concerns is having this in *every* app folder. You mentioned versioning as one driver for this, and while this would've been really nice for shared/ code, do you really think it's necessary for web components? They are way more sandboxed than previous shared building blocks, so I feel like it's slightly less necessary. Are there any other reasons for it? A few of the reasons I'm not too happy with it: 1 - Bloats the repo size (tiny right now, but potentially a larger issue in the future). 2 - 'Grepability' of apps will suffer dramatically. I feel a fairly common thing to do is to grep the entire apps/ folder looking for matches when you are doing cross-app changes. It may seem a little silly to worry about this - but I ensure you it could impact many developers negatively. 3 - Makes pull requests much harder to read and review with all of the extra code in there. Don't get me wrong, I'm totally happy with this - I just have a few concerns about putting bower stuff into each app (instead of into shared/ for example). Let me know what you think.
Flags: needinfo?(wilsonpage)
I agree that there are some small irritations to work-flow that may come with this change, but stepping back and looking at the benefits to the project as a whole, they begin to seem insignificant: 1. We enable third party app developers to gain access to very high quality components, empowering them to build excellent apps with ease. This benefits the FirefoxOS eco-system, which at the end of the day will make or break this entire project. Building good components is hard! 2. We enable other Mozilla projects (such as Brick and WebMaker) to use/extend Gaia components, fostering the FirefoxOS developer community. 3. Versioning enables component owners can make big changes/improvements to components without fear of breaking 20 apps in one go (remember v1.3?). I agree WC encapsulation will help with this too, but somewhere along the line big changes will need to be made. If component owners can make big changes and experiment freely in a few apps at a time, we can iterate as an OS far more quickly. 4. Components can declare sub-dependencies. Currently if a shared component depends on another library, it requires that the user needs to supply this, or everything breaks. We have no way to say, 'this library depends on this library at this version', and that exact dependency get delivered. Users needs to inspect the internals of a library to work out if/what it depends on, then go and find it. 5. We empower the app owners to take charge of their dependencies and move away from parsing the <head> to determine what an app needs. Removing build step magic and hacks when this magic doesn't fulfil requirements (see <head> in Camera). 6. We encourage app owners to use third party tools and libraries. Gaia is a small resource tight team. Any time we can save by using strong battle-tested solutions is a big win for us. Reaching for an established library instead of a developer rolling their own, although against some Gaia devs mantra, could save days/weeks/months of Mozilla developer time. Obviously there's a time and a place for this. We're not encouraging the use of jQuery, but instead smaller/targeted libraries, great at doing one thing. `$ bower install <great-library>` 7. We're moving closer to a work-flow that web-developers understand. I remember looking at Gaia for the first time and being baffled at all the files in the <head> and not having a clue where they came from. If Gaia apps are easier to understand I feel it will lower the perceived barrier to entry for third party app developers. --- This decision is about weighing up high level pros and cons and less our internal work-flow niggles. I personally feel this is an opportunity to ignite FirefoxOS development and innovation. SHARING to feed the apps ecosystem and BORROWING to make our small team more competitive.
Flags: needinfo?(wilsonpage)
Attachment #8438402 - Flags: review?(21)
jrburke suggested that we don't actually have to force one particular tool (like Bower), but instead allow app owners to decide which tool (Bower, NPM, Volo, Repo, etc) they wish to use to pull their package dependencies in. He stressed that Gaia should move towards a 'pull based' model to empower app owners, and reduce/remove inter-app dependencies. NI jrburke to chime-in / correct.
Flags: needinfo?(jrburke)
A lot of the initial discussion happened over on bug 1022589, so head over there if you're late to the party.
That sounds like the correct summary of my comments. As we can push more apps to be privileged and even have deployment lives outside of Gaia, directly to the marketplace for example, allowing apps to pull in the versions of dependencies will be more useful. This is generally how dependencies work anyway for standalone web apps, the desired version is pull in vs. pushed by a common code area. This also frees the people working on shared components to move at a faster pace than apps, to do alternate branches and such. It would be great to get to a point where gaia is just a curated pull of git commit hashes from individual app repos. There is a lot tangled up with it (l10n for example), but as to the choice of what tool is used to pull, as long as the app persists what hash IDs/version numbers of dependencies are in use, that seems to be the minimum bar. If the app commits the dependencies in with the app, the app's choice of pull tool is not foisted on others. That seemed like an alternate pathway to get to a pull model rather than having everyone agree on a specific pull tool. I can appreciate the grep issue, but also, hopefully the grep tool is good enough to allow directory exclusions. I am sure there are still some rough edges with that, but all the possible solutions have tradeoffs.
Flags: needinfo?(jrburke)
Comment on attachment 8438402 [details] [review] pull-request (master) Overall, it looks good. A few small things I would like to address before landing this: - s/bower_components/components I don't mind that we use bower or something else in apps. But at least lets have a common structure in apps, so people may decide to switch at some point to something else, without having the 'tool' naming leaking around. - http://github.com/gaia-components -> http://github.com/mozilla-b2g/gaia-components Let's not create a new repo, with a new policy of access, etc... Also there is one thing I want to double check to ensure we are on the same line before doing that, in order to make sure this work fits well with bug 1011738: The usage of css variables, and its fallback mechanism, I can see in bug 1022589 for the gaia-header component looks great - but it should probably be a separate patch/bug/issue. On this topic, it makes perfect sense that components have a fallback look and feel if there is no theme set, but I want to make sure we apply that everywhere it makes sense. Like colors, font-family, icons, images, etc... Using native support of css variables for that sounds good for me AFAICT with the current use case we have. I just want to stress a bit on that, since from what I have seen in bug 1022589 and this bug, since I'm not sure how people are supposed to override Gaia-Icons or the gradient image in those patches. So for Gaia-Icons and theming in general it would be nice nice to think about it more (even by discussing on irc, on a separate bug, etc...) to see how they are going to work. That said, I don't think this is a blocker for this bug to go forward. Then if we end up that the size of the repo is an issue in the future (and it will likely be), we will find some solutions - no need to block on that as well. Fwiw and for a long time, I felt a bit like Kevin and looked for a centralized solution in shared/. I was hoping that this 'versioning' could probably be done in shared/ and apps that needs an older version, can just make a local copy of the building block / component / etc. It never really works, so trying this which is a sort of similar but contrary approach sounds good.
Attachment #8438402 - Flags: review?(21)
Comment on attachment 8438402 [details] [review] pull-request (master) In case this is not obvious, this is a f+. :)
Attachment #8438402 - Flags: feedback+
(In reply to Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please) from comment #9) > Comment on attachment 8438402 [details] [review] > pull-request (master) > > Overall, it looks good. > > A few small things I would like to address before landing this: > - s/bower_components/components > I don't mind that we use bower or something else in apps. But at least > lets have a common structure in apps, so people may decide to switch at some > point to something else, without having the 'tool' naming leaking around. I know that it's possible to change `bower_components` directory name, but if the user decided to use a different tool to resolve their dependencies (such as NPM), I'm not sure you are able to control the name on the packages directory. > - http://github.com/gaia-components -> > http://github.com/mozilla-b2g/gaia-components > Let's not create a new repo, with a new policy of access, etc... I know Kevin feels quite strongly about having these components in a separate organization to mozilla-b2g. I don't really mind that much. If we do migrate to 'mozilla-b2g', myself and other component authors will need permissions to create new repos. I will continue to work out of 'gaia-components' organization until we have agreed on this. > > Also there is one thing I want to double check to ensure we are on the same > line before doing that, in order to make sure this work fits well with bug > 1011738: > > The usage of css variables, and its fallback mechanism, I can see in bug > 1022589 for the gaia-header component looks great - but it should probably > be a separate patch/bug/issue. > > On this topic, it makes perfect sense that components have a fallback look > and feel if there is no theme set, but I want to make sure we apply that > everywhere it makes sense. Like colors, font-family, icons, images, etc... > > Using native support of css variables for that sounds good for me AFAICT > with the current use case we have. > > I just want to stress a bit on that, since from what I have seen in bug > 1022589 and this bug, since I'm not sure how people are supposed to override > Gaia-Icons or the gradient image in those patches. > > So for Gaia-Icons and theming in general it would be nice nice to think > about it more (even by discussing on irc, on a separate bug, etc...) to see > how they are going to work. That said, I don't think this is a blocker for > this bug to go forward. Myself an Casey (cyee) has been discussing the theming side of components in quite some detail. You're right that currently the icons and component's internal images cannot be overridden by an outside `theme.css`. In order to get that functionality we will need to have the `/deep/` selectors available on the platform. This means that, as well as CSS variables, a theme could also have /deep/ selectors that can modify icons/images behind the shadow-boundary, something like: /deep/ .icon-camera { ... } > Then if we end up that the size of the repo is an issue in the future (and > it will likely be), we will find some solutions - no need to block on that > as well. > > > Fwiw and for a long time, I felt a bit like Kevin and looked for a > centralized solution in shared/. I was hoping that this 'versioning' could > probably be done in shared/ and apps that needs an older version, can just > make a local copy of the building block / component / etc. It never really > works, so trying this which is a sort of similar but contrary approach > sounds good. In case it's not obvious, I'm MEGA HAPPY you're on-board! Thanks for your feedback :)
Assignee: wilsonpage → nobody
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: