#246 Add issues and links to issues in release notes
Merged 2 years ago by thunderbirdtr. Opened 2 years ago by meltayeb.
fedora-web/ meltayeb/websites ReleaseNotes  into  master

@@ -13,7 +13,7 @@ 

  

  const baseProdUrl = 'https://builds.coreos.fedoraproject.org/prod/streams'

  const baseDevelUrl = 'https://builds.coreos.fedoraproject.org/devel/streams'

- 

+ const baseReleaseNoteUrl = 'https://builds.coreos.fedoraproject.org/release-notes'

  const initialBuildsShown = 5;

  

  // pkgdiff enum to str
@@ -29,6 +29,10 @@ 

    return `${month} ${day}, ${year}`;

  }

  

+ function fetchReleaseNotes(releaseStream) {

+   return fetch(`${baseReleaseNoteUrl}/${releaseStream}.json`)

+     .then(response => response.ok ? response.json() : {});

+ }

  function getBaseUrl(stream, developer) {

    return stream != "developer"

      ? `${baseProdUrl}/${stream}`
@@ -271,8 +275,8 @@ 

      var finalCommitMeta = {}

      let promises = []

      // Fetch meta and commitmeta for each architecture for the build

-     for (const metaForEachArch of metaList[0]){

-       let basearch= metaForEachArch[0];

+     for (const metaForEachArch of metaList[0]) {

+       let basearch = metaForEachArch[0];

        // Adding meta for each architecture

        meta[basearch] = metaForEachArch[1]

        // check if `parent-pkgdiff` field is present, if present there's no need to manually
@@ -296,7 +300,7 @@ 

        }));

      }

      // Return a single promise when all the promises get resolved

-     return Promise.all(promises)  

+     return Promise.all(promises)

    });

  }

  
@@ -307,8 +311,8 @@ 

        .then(response => Promise.all([build.arches[0], response.ok ? response.json() : {}]));

    }

    return Promise.all(build.arches.map(arch => {

-       return fetch(`${base}/${build.id}/${arch}/meta.json`)

-           .then(resp => Promise.all([arch, resp.ok ? resp.json() : {}]));

+     return fetch(`${base}/${build.id}/${arch}/meta.json`)

+       .then(resp => Promise.all([arch, resp.ok ? resp.json() : {}]));

    }));

  }

  
@@ -322,6 +326,7 @@ 

      .then(response => response.ok ? response.json() : {});

  }

  

+ 

  var coreos_release_notes = new Vue({

    el: '#coreos-release-notes',

    created: function () { this.refreshBuilds() },
@@ -342,6 +347,8 @@ 

      // of the first arch, but in the future these would be e.g.

      // meta[arch] and commitmeta[arch]

      releases: [],

+     //holds the release note issues for the chosen stream

+     issues: [],

      // list of unshown {id, arches, meta, commitmeta} build objects

      unshown_builds: [],

      // toggles "Loading..."
@@ -401,7 +408,6 @@ 

        if (self.loading) {

          return;

        }

- 

        rows = [];

        self.releases.forEach((build, idx) => {

          let selectedArch = build.selectedArch
@@ -418,60 +424,82 @@ 

            headingListArches.push(h('h6', {}, arch));

          });

  

-         archDropdown = h('div', {attrs: {style: "text-align: left;" }}, [

+         archDropdown = h('div', { attrs: { style: "text-align: left;" } }, [

            `Arch: `,

-             h('a', {

-               class: "dropdown-toggle",

-               attrs: {

-                 "id": build.id+"Dropdown",

-                 "href": "#",

-                 "role": "button",

-                 "data-toggle": "dropdown",

-                 "aria-haspopup": true,

-                 "aria-expanded": false,

-               },

-               on: {

-                 click: function (e) {

-                   e.preventDefault();

-                 }

+           h('a', {

+             class: "dropdown-toggle",

+             attrs: {

+               "id": build.id + "Dropdown",

+               "href": "#",

+               "role": "button",

+               "data-toggle": "dropdown",

+               "aria-haspopup": true,

+               "aria-expanded": false,

+             },

+             on: {

+               click: function (e) {

+                 e.preventDefault();

                }

-             }, selectedArch),

-             h('div', { class: "dropdown-menu" }, [

-               h('div', { class: "container" }, [

-                   h('div', { class: "col-12 px-0" }, [

-                     Object.entries(build.arches).map(pair => {

-                       let arch = pair[1];

-                       return h('a', {

-                         class: "dropdown-item",

-                         attrs: {

-                           href: "#",

-                         },

-                         on: {

-                           click: function (e) {

-                             e.preventDefault();

-                             Object.entries(build.arches).map(pair => {

-                               if(pair[1] == e.target.text)

-                                 document.getElementById(build.id+pair[1]).hidden = false

-                               else

-                                 document.getElementById(build.id+pair[1]).hidden = true

-                             });

-                             document.getElementById(build.id+"Dropdown").text = e.target.text;

-                           }

-                         }

-                       },

-                         arch);

-                     })

-                   ])

+             }

+           }, selectedArch),

+           h('div', { class: "dropdown-menu" }, [

+             h('div', { class: "container" }, [

+               h('div', { class: "col-12 px-0" }, [

+                 Object.entries(build.arches).map(pair => {

+                   let arch = pair[1];

+                   return h('a', {

+                     class: "dropdown-item",

+                     attrs: {

+                       href: "#",

+                     },

+                     on: {

+                       click: function (e) {

+                         e.preventDefault();

+                         Object.entries(build.arches).map(pair => {

+                           if (pair[1] == e.target.text)

+                             document.getElementById(build.id + pair[1]).hidden = false

+                           else

+                             document.getElementById(build.id + pair[1]).hidden = true

+                         });

+                         document.getElementById(build.id + "Dropdown").text = e.target.text;

+                       }

+                     }

+                   },

+                     arch);

+                 })

                ])

-             ]),

+             ])

+           ]),

          ]);

          let leftPane = h('div', { class: "col-lg-2" }, [headingBuildId, archDropdown]);

- 

-         //Adding the information for all arches to rightPane but only showing the card for the selected arch in dropdown

+         // Adding the information for all arches to rightPane but only showing the card for the selected arch in dropdown

          let rightPane = [];

          build.arches.forEach(eachArch => {

            // Right pane consists of detailed package information

            let date = h('p', {}, `Release Date: ${timestampToPrettyString(build.meta[eachArch]['coreos-assembler.build-timestamp'])}`);

+           // title of release issues if the release notes don't exist

+           let releaseNoteTitle = h('h6');

+           // holds elements of release notes to be rendered

+           let releaseNotesElements = [];

+           // get the keys from the JSON

+           releasesJson = this.issues.releases;

+           // if the release notes exist in the json

+           if (releasesJson.hasOwnProperty(build.id)) {

+             if (releasesJson[build.id].issues.length === 0) {

+               // if the release notes exist but there are no issues provided

+               releaseNoteTitle = h('h6', {}, `No specific issues fixed in this release.`);

+             } else {

+               // if the release notes exist and there are issues provided

+               releaseNoteTitle = h('h6', {}, `Issues fixed:`);

+               specificIssue = releasesJson[build.id].issues;

+               let releaseNotesLinkAndText = h('ul', specificIssue?.map(({ url, text }) => {

+                 return h('li', [h('a', { attrs: { href: url }, }, text)]);

+               }));

+               releaseNotesElements.push(releaseNotesLinkAndText);

+             }

+           } else {

+             releaseNoteTitle = h('h6', {}, `Releases notes for this release are still pending review.`);

+           }

            // List of important packages and versions

            let importantPkgsElements = [];

            build.commitmeta[eachArch].importantPkgs.forEach((pkg, _) => {
@@ -686,11 +714,11 @@ 

              downgradedPkgsHeading = h('p', { class: "mt-3" }, "Downgraded:");

            }

            let downgradedPkgsElements = h('div', { attrs: { hidden: true } }, [downgradedPkgsHeading, h('ul', {}, downgradedPkgsElementsList)]);

-           let rightPaneData = h('div', { attrs: {id: build.id+eachArch}, class: "col-lg-10 border-bottom mb-5 pb-4" }, 

-             [date, importantPkgsElements, pkgSummaryDiv, totalPkgsElements, addedPkgsElements, removedPkgsElements, upgradedPkgsElements, downgradedPkgsElements]);

-           

-             // Hiding the information cards of the unselected architectures

-           if(eachArch!=selectedArch)

+           let rightPaneData = h('div', { attrs: { id: build.id + eachArch }, class: "col-lg-10 border-bottom mb-5 pb-4" },

+             [date, releaseNoteTitle, releaseNotesElements, importantPkgsElements, pkgSummaryDiv, totalPkgsElements, addedPkgsElements, removedPkgsElements, upgradedPkgsElements, downgradedPkgsElements]);

+ 

+           // Hiding the information cards of the unselected architectures

+           if (eachArch != selectedArch)

              rightPaneData.data.attrs.hidden = true

            rightPane.push(rightPaneData)

          });
@@ -703,6 +731,10 @@ 

        this.loading = true

        this.releasesUrl = getBaseUrl(this.stream, this.developer);

        this.buildsUrl = getBaseUrl(this.stream, this.developer) + "/builds";

+       this.issues = [];

+       fetchReleaseNotes(this.stream).then((issuesJson) => {

+         this.issues = issuesJson;

+       });

        fetchReleases(this.releasesUrl).then(releaseVersions => {

          fetchBuilds(this.buildsUrl).then(result => {

            [legacy, builds] = result;
@@ -710,6 +742,7 @@ 

            this.legacy = legacy;

            this.releases = [];

            this.unshown_builds = [];

+ 

            // counter for the number of release metadata fetched since fetch is asnyc operation

            let counter = 0;

  

Add issues from JSON and links to those issues in CoreOS release notes. Also add indentation.

LGTM. Is this fetching all streams at once? Could we fetch only the stream we need as required?

2 new commits added

  • Add issues and links to issues in release notes
  • Add indentation for coreos-release-notes.js
2 years ago

1 new commit added

  • Add issues and links to issues in release notes
2 years ago

2 new commits added

  • Add issues and links to issues in release notes
  • Add indentation for coreos-release-notes.js
2 years ago

1 new commit added

  • Add issues and links to issues in release notes
2 years ago

2 new commits added

  • Add issues and links to issues in release notes
  • Add indentation for coreos-release-notes.js
2 years ago

1 new commit added

  • Fetch Release Issues JSON From Chosen Stream
2 years ago

1 new commit added

  • Small issues
2 years ago

2 new commits added

  • Add issues and links to issues in release notes
  • Add indentation for coreos-release-notes.js
2 years ago

@siosm

LGTM. Is this fetching all streams at once? Could we fetch only the stream we need as required?

It now fetches from the single specified stream :thumbsup:

Two minor suggestions:
- Could you exclude the dots ('*') from the link anchor for each issue to make it more easier to read?
- If there are no issues for a given release, could we not display the 'Issues fixes' text?

Thanks!

Metadata Update from @thunderbirdtr:
- Request assigned

2 years ago

1 new commit added

  • Add functionality to make title appear if release issues exists
2 years ago

1 new commit added

  • Small changes removing function
2 years ago

2 new commits added

  • Add issues and links to issues in release notes
  • Add indentation for coreos-release-notes.js
2 years ago

1 new commit added

  • Small changes adding new function
2 years ago

2 new commits added

  • Add issues and links to issues in release notes
  • Add indentation for coreos-release-notes.js
2 years ago

1 new commit added

  • Bug fixes
2 years ago

2 new commits added

  • Add issues and links to issues in release notes
  • Add indentation for coreos-release-notes.js
2 years ago

@thunderbirdtr Could you have a look at this and possibly merge it? Thanks.

LGTM to me. @siosm could you do one more final check then I'll merge it.

if (releasesJson.hasOwnProperty(build.id)) {

It looks like you do this check once for the switch case and then once again below. One should be enough?

I think we should have the following:
- if the build id does not exists in the JSON, then add something like "Releases notes for this release are still pending review"
- If the build id exists in the JSON but the list is empty, then display: " No specific issues fixed in this release".
- If the build id exists and there are issues then we list them.

cc @dustymabe @jlebon WDYT about this? I can re-add the missing empty buildids to the notes.


  • Could you exclude the dots ('*') from the link anchor for each issue to make it more easier to read?

Sorry, maybe this was ambiguous. Could we make this a proper HTML list with entries so that they get their own "dots" but not as part of the link?

+                          let releaseNotesLinkAndText = h('a', { attrs: { href: specificIssue[i].url }, }, specificIssue[i].text);            
                    +                          releaseNotesElements.push(releaseNotesLinkAndText);            
                    +                          releaseNotesElements.push(h('br'));

// TODO: naive implementation is a list of subjects under each component header            
// in the future add buttons for detailed information of each note item

I'd prefer we track future improvement in the parent issue as TODOs in the code here will be forgotten.

1 new commit added

  • Added lists to release note issues
2 years ago

This will likely never be reached given that we have the same initial check in 189

I don't think we need this one anymore. It looks more compact without it.

In this case, we have three options: the non-existing path, the existing & empty one, and the existing and not empty one. I think it will be easier to check for existence first, then if the list is empty.

2 new commits added

  • Add issues and links to issues in release notes
  • Add indentation for coreos-release-notes.js
2 years ago

Minor nit: We're losing this comment here. Maybe it should we re-added below the code added here.

I think you probably need an else branch here now. Unfortunately we don't have an example to try this one out.

Minor nit: Let's keep this empty line above the comment

2 new commits added

  • Add issues and links to issues in release notes
  • Add indentation for coreos-release-notes.js
2 years ago

2 new commits added

  • Add issues and links to issues in release notes
  • Add indentation for coreos-release-notes.js
2 years ago

2 new commits added

  • Add issues and links to issues in release notes
  • Add indentation for coreos-release-notes.js
2 years ago

2 new commits added

  • Add issues and links to issues in release notes
  • Add indentation for coreos-release-notes.js
2 years ago

This looks great to me! Thanks a lot! Let's merge this @thunderbirdtr please! :)

Pull-Request has been merged by thunderbirdtr

2 years ago

@siosm @meltayeb thank you guys !!

Merge complete