mozilla-frontend-infra / firefox-health-dashboard

https://firefox-health-dashboard.netlify.com
Mozilla Public License 2.0
26 stars 68 forks source link

Ensure config files have link to repo #643

Closed klahnakoski closed 3 years ago

klahnakoski commented 4 years ago

The links to revisions can take a different form, depending on the repository it is found in:

    const revisionURL = record.meta.repo === 'mozilla-central'
      ? `https://hg.mozilla.org/mozilla-central/pushloghtml?changeset=${record.revision}`
      : `https://github.com/mozilla-mobile/fenix/commit/${record.revision}`;

Ensure we have this these URL templates in the config files, so the code is not making decisions:

    const revisionURL = URL(record.meta.repo.revisionURL, {changeset: record.revision})

Specifically, put a breakpoint on the above lines, then load

https://localhost:5000/playback/details?platform=p2-aarch64&browser=fenix&encoding=VP9&past=week&ending=2020-01-07

to see what record.meta looks like. You can see the same in the code:

https://github.com/mozilla-frontend-infra/firefox-health-dashboard/blob/a04a6f62f5524968a5678460ce6fd6a32323e275/src/playback/config.js#L128

Replace the repo: 'mozilla-central' property with repo: MOZILLA_CENTRAL, where MOZILLA_CENTRAL is defined as

MOZILLA_CENTRAL = {
    name: "mozilla-central",
    revisionURL: "https://hg.mozilla.org/mozilla-central/pushloghtml",
    // plus some other properties
}

Do the same thing for FENIX, and adjust the code so it works with the new structure.

At a minimum, do this for /playback. Bonus points if you do this everywhere "mozilla-central" is found.

StTronn commented 4 years ago

Hey @klahnakoski I will like to take this issue and start with the cleanup for /playback

klahnakoski commented 4 years ago

@StTronn Yes please. Thank you!

colibie commented 4 years ago

Hi, I'm from Outreachy and would like to know if this issue is still unresolved so I can work on it.

kimberlythegeek commented 4 years ago

@StTronn are you still working on this?

StTronn commented 4 years ago

hey, I looked at the issue again and I think it is only partially resolved. It seems that the url templates are needed to be added everywhere while I have done it only in /playback/ #647. @kimberlythegeek I am currently not working on this.

kimberlythegeek commented 4 years ago

@StTronn thanks for the update!

colibie commented 4 years ago

Hi @kimberlythegeek, I've sent a PR for this issue.