Closed Bug 1454827 Opened 3 years ago Closed 1 year ago

Source actor spec should use `source` instead of `onSource`

Categories

(DevTools :: General, enhancement, P3)

enhancement

Tracking

(firefox70 fixed)

RESOLVED FIXED
Firefox 70
Tracking Status
firefox70 --- fixed

People

(Reporter: jryans, Assigned: malikanshul29)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

Source actor's spec includes:

    onSource: {
      request: { type: "source" },
      response: RetVal("json")
    },

which is unusual, since the request's type would be "source" if the method were renamed to that.  We should explore renaming the method and removing the type to match typical style.
Product: Firefox → DevTools
Assignee: nobody → mtigley
Status: NEW → ASSIGNED
Comment on attachment 8991100 [details]
Bug 1454827 - Source actor spec should use `source` instead of `onSource`.

https://reviewboard.mozilla.org/r/256090/#review263048

Thanks a lot for thaking this on Micah! I realized too late that the name I suggested to you wasn't the best one, so I wrote a comment regarding that. Otherwise looks really good.

::: commit-message-6073e:1
(Diff revision 1)
> +Bug 1454827 - Source actor spec should use `source` instead of `onSource`. r=jryans,yulia

nit: we usually write what was done in the commit rather than the bug name: in this case "replace onSource with source in Source Actor and Source Actor Spec"

::: devtools/server/actors/source.js:190
(Diff revision 1)
>      return this._threadActor.sources;
>    },
>    get dbg() {
>      return this.threadActor.dbg;
>    },
> -  get source() {
> +  get sourceData() {

I just realized that a better name for this is `originalSource`. 

We have two types of source that we can get. `generatedSource` is a compiled source, like something you get out of webpack when you bundle. `originalSource` is a representation of the file that a programmer was working on when they were writing their code.

I would propose that we rename sourceData to originalSource, because then we are consistant with generatedSource. Sorry it took me a while to think of that!
Attachment #8991100 - Flags: review?(ystartsev) → review+
Comment on attachment 8991100 [details]
Bug 1454827 - Source actor spec should use `source` instead of `onSource`.

https://reviewboard.mozilla.org/r/256090/#review263062

Thanks for working on this! :) I agree with Yulia's comments.
Attachment #8991100 - Flags: review?(jryans) → review+

This bug has not been updated in the last 6 months. Resetting the assignee field.
Please, feel free to pick it up again and add a comment outlining your plans for it if you do still intend to work on it.
This is just trying to clean our backlog of bugs and make bugs available for people.

Assignee: mtigley → nobody
Status: ASSIGNED → NEW
Assignee: nobody → malikanshul29
Pushed by ystartsev@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5f00d5f48cb2
Rename onSource to source in source actor r=yulia
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 70
You need to log in before you can comment on or make changes to this bug.