Closed
Bug 1283123
Opened 8 years ago
Closed 8 years ago
Reps: make it possible to pass in a component to handle object links
Categories
(DevTools :: Shared Components, defect, P1)
DevTools
Shared Components
Tracking
(firefox50 fixed)
Tracking | Status | |
---|---|---|
firefox50 | --- | fixed |
People
(Reporter: linclark, Assigned: nchevobbe)
References
Details
(Whiteboard: [devtools-html])
Attachments
(1 file, 2 obsolete files)
This is useful for making objects link to variables view.
Reporter | ||
Comment 1•8 years ago
|
||
Reporter | ||
Updated•8 years ago
|
Attachment #8766339 -
Attachment is obsolete: true
Reporter | ||
Comment 2•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/61420/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/61420/
Attachment #8766586 -
Flags: review?(odvarko)
Attachment #8766586 -
Flags: review?(lclark)
Reporter | ||
Updated•8 years ago
|
Whiteboard: [devtools-html] [triage]
Updated•8 years ago
|
Blocks: devtools-html-2
Updated•8 years ago
|
Flags: qe-verify-
Priority: -- → P2
Whiteboard: [devtools-html] [triage] → [devtools-html]
Updated•8 years ago
|
Iteration: --- → 50.2 - Jul 4
Priority: P2 → P1
Reporter | ||
Updated•8 years ago
|
Assignee: nobody → chevobbe.nicolas
Updated•8 years ago
|
Status: NEW → ASSIGNED
Reporter | ||
Comment 3•8 years ago
|
||
Comment on attachment 8766586 [details]
Bug 1283123 - Reps: make it possible to pass in a component to handle object links.
https://reviewboard.mozilla.org/r/61420/#review58562
This is passing all committed tests, so r+ from me.
Attachment #8766586 -
Flags: review?(lclark) → review+
Reporter | ||
Comment 4•8 years ago
|
||
Actually, one thing I thought of... we might want the property passed in to be "grip" rather than "objectActor". Honza, what do you think?
Flags: needinfo?(odvarko)
Comment 5•8 years ago
|
||
Comment on attachment 8766586 [details]
Bug 1283123 - Reps: make it possible to pass in a component to handle object links.
https://reviewboard.mozilla.org/r/61420/#review58590
Great work here!
(In reply to Lin Clark [:linclark] from comment #4)
> Actually, one thing I thought of... we might want the property passed in to
> be "grip" rather than "objectActor". Honza, what do you think?
Yes agree the name isn't clear (an actor is living on the backend not on the front-end).
I think we can use yet more generic name: `object`. The value that is passed to the objectLink component is always the `object` property from the parent rep. It's the same thing and it should have the same name.
Pleas fix it and I'll quickly re-review again.
Thanks!
Honza
Attachment #8766586 -
Flags: review?(odvarko)
Reporter | ||
Updated•8 years ago
|
Attachment #8766586 -
Attachment is obsolete: true
Assignee | ||
Comment 6•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/61956/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/61956/
Attachment #8767484 -
Flags: review?(odvarko)
Attachment #8767484 -
Flags: review?(lclark)
Updated•8 years ago
|
Attachment #8767484 -
Flags: review?(odvarko) → review+
Comment 7•8 years ago
|
||
Comment on attachment 8767484 [details]
Bug 1283123 - Reps: make it possible to pass in a component to handle object links.
https://reviewboard.mozilla.org/r/61956/#review58796
Looks good to me.
Honza
Updated•8 years ago
|
Flags: needinfo?(odvarko)
Assignee | ||
Comment 8•8 years ago
|
||
Comment on attachment 8767484 [details]
Bug 1283123 - Reps: make it possible to pass in a component to handle object links.
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/61956/diff/1-2/
Updated•8 years ago
|
Iteration: 50.2 - Jul 4 → 50.3 - Jul 18
Reporter | ||
Comment 9•8 years ago
|
||
Comment on attachment 8767484 [details]
Bug 1283123 - Reps: make it possible to pass in a component to handle object links.
https://reviewboard.mozilla.org/r/61956/#review59350
This looks good to me, but unfortunately will need an update. The changes to the DatePreview component in new console frontend won't apply because that has been changed with Bug 1283870.
Attachment #8767484 -
Flags: review?(lclark)
Assignee | ||
Comment 10•8 years ago
|
||
Comment on attachment 8767484 [details]
Bug 1283123 - Reps: make it possible to pass in a component to handle object links.
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/61956/diff/2-3/
Attachment #8767484 -
Flags: review?(lclark)
Assignee | ||
Comment 11•8 years ago
|
||
(In reply to Nicolas Chevobbe [:nchevobbe] from comment #10)
> Comment on attachment 8767484 [details]
> Bug 1283123 - Reps: make it possible to pass in a component to handle object
> links.
>
> Review request updated; see interdiff:
> https://reviewboard.mozilla.org/r/61956/diff/2-3/
Pulled and rebased.
date-preview.js should not be in the patch anymore
Reporter | ||
Comment 12•8 years ago
|
||
Comment on attachment 8767484 [details]
Bug 1283123 - Reps: make it possible to pass in a component to handle object links.
https://reviewboard.mozilla.org/r/61956/#review59370
LGTM, thanks!
Attachment #8767484 -
Flags: review?(lclark) → review+
Assignee | ||
Comment 13•8 years ago
|
||
TRY is green https://treeherder.mozilla.org/#/jobs?repo=try&revision=54bb82c5ba1b&selectedJob=23386583 except an Android checkstyle (sucessfully retriggered though) which does not seem caused by my patch.
Keywords: checkin-needed
Comment 14•8 years ago
|
||
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/fx-team/rev/79d1579dee2e
Reps: make it possible to pass in a component to handle object links. r=linclark,honza
Keywords: checkin-needed
Comment 15•8 years ago
|
||
sorry had to back this out for test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=10338699&repo=fx-team
Flags: needinfo?(chevobbe.nicolas)
Comment 16•8 years ago
|
||
Backout by cbook@mozilla.com:
https://hg.mozilla.org/integration/fx-team/rev/fa4355015e36
Backed out changeset 79d1579dee2e for test failures in test_reps_date-time.html
Assignee | ||
Comment 17•8 years ago
|
||
Okay, I see what's wrong in my patch, I'll push again in a couple hours.
Any idea why my TRY push ( https://treeherder.mozilla.org/#/jobs?repo=try&revision=54bb82c5ba1b&selectedJob=23386583 ) didn't show this failure ? Does the try syntax doesn't include this test ?
Flags: needinfo?(chevobbe.nicolas) → needinfo?(cbook)
Comment 18•8 years ago
|
||
(In reply to Nicolas Chevobbe [:nchevobbe] from comment #17)
> Okay, I see what's wrong in my patch, I'll push again in a couple hours.
> Any idea why my TRY push (
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=54bb82c5ba1b&selectedJob=23386583 ) didn't show this
> failure ? Does the try syntax doesn't include this test ?
yeah seems the test failures where in [TC] Linux64 mochitest-chrome-1 and seems that was not included in the try, also don't worry that kind of things happen :))))
Flags: needinfo?(cbook)
Assignee | ||
Comment 19•8 years ago
|
||
Comment on attachment 8767484 [details]
Bug 1283123 - Reps: make it possible to pass in a component to handle object links.
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/61956/diff/3-4/
Assignee | ||
Comment 20•8 years ago
|
||
Comment on attachment 8767484 [details]
Bug 1283123 - Reps: make it possible to pass in a component to handle object links.
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/61956/diff/4-5/
Assignee | ||
Comment 21•8 years ago
|
||
Comment on attachment 8767484 [details]
Bug 1283123 - Reps: make it possible to pass in a component to handle object links.
My patch did not pass the Rep's tests (which are not in dt- , didn't knew that). I made some changes in order to pass the test, which it does now.
Attachment #8767484 -
Flags: review+ → review?(lclark)
Reporter | ||
Comment 22•8 years ago
|
||
Comment on attachment 8767484 [details]
Bug 1283123 - Reps: make it possible to pass in a component to handle object links.
https://reviewboard.mozilla.org/r/61956/#review59804
Attachment #8767484 -
Flags: review?(lclark) → review+
Assignee | ||
Comment 23•8 years ago
|
||
Comment on attachment 8767484 [details]
Bug 1283123 - Reps: make it possible to pass in a component to handle object links.
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/61956/diff/5-6/
Assignee | ||
Comment 24•8 years ago
|
||
Comment on attachment 8767484 [details]
Bug 1283123 - Reps: make it possible to pass in a component to handle object links.
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/61956/diff/6-7/
Assignee | ||
Comment 25•8 years ago
|
||
TRY is green https://treeherder.mozilla.org/#/jobs?repo=try&revision=d95bca8ad9fc , except from some known intermittents, unrelated to my patch.
Keywords: checkin-needed
Comment 26•8 years ago
|
||
has problems to apply:
patching file devtools/client/shared/components/reps/grip-array.js
Hunk #4 FAILED at 125
1 out of 4 hunks FAILED -- saving rejects to file devtools/client/shared/components/reps/grip-array.js.rej
nicolas, could you take a look again ? Thanks!
Flags: needinfo?(chevobbe.nicolas)
Updated•8 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 27•8 years ago
|
||
Comment on attachment 8767484 [details]
Bug 1283123 - Reps: make it possible to pass in a component to handle object links.
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/61956/diff/7-8/
Assignee | ||
Comment 28•8 years ago
|
||
Rebased and merged, should be good
Flags: needinfo?(chevobbe.nicolas)
Keywords: checkin-needed
Blocks: 1281047
Comment 29•8 years ago
|
||
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/fx-team/rev/5b767b7a32d4
Reps: make it possible to pass in a component to handle object links. r=linclark,honza
Keywords: checkin-needed
Comment 30•8 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 50
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•