PR#26: Redundancy removed from Blockers - fedora-qa/landingpage

fedora-qa / landingpage

#26 Redundancy removed from Blockers

Closed 2 years ago by lbrabec. Opened 3 years ago by alisina92.

fedora-qa/ alisina92/landingpage Redundancy-reduced-in-Blockers-Alisina into master

fixed the redundancy

[Alisina92] • 3 years ago

fc0d71b

Redundancy removed from Blockers

[Alisina92] • 3 years ago

e554aa9

src/landingpage/Blockers.js

file modified

+21 -81

		`@@ -1,85 +1,25 @@`
		`- import React, { Component } from "react"`
		`- import { Row } from "reactstrap"`
		`- import SourceLink from "./SourceLink"`
		`+ import React, { useState } from "react";`
		`+ import { Row } from "reactstrap";`
		`+ import SourceLink from "./SourceLink";`
		`+ import BugStats from "./BugStats";`

		`- class Blockers extends Component {`
		`- constructor(props) {`
		`- super(props)`
		`- this.state = { blockerbugs: {}, release: 0 }`
		`- }`

		`- render() {`
		`- if (this.props.data)`
		`- return (`
		`- <div>`
		`- <h1 className="padded">`
		`- Fedora {this.props.release} blockers and FEs{" "}`
		`- <SourceLink href="https://qa.fedoraproject.org/blockerbugs/" />`
		`- </h1>`
		`- <Row>`
		`- <div className="col-md-6">`
		`- <table className="events">`
		`- <thead>`
		`- <tr>`
		`- <td colSpan="2">`
		`- <b>Beta</b>`
		`- </td>`
		`- </tr>`
		`- </thead>`
		`- <tbody>`
		`- <tr>`
		`- <td>Proposed Blockers</td>`
		`- <td className="bugscount">{this.props.data.beta_blockers_proposed}</td>`
		`- </tr>`
		`- <tr>`
		`- <td>Accepted Blockers</td>`
		`- <td className="bugscount">{this.props.data.beta_blockers}</td>`
		`- </tr>`
		`- <tr>`
		`- <td>Proposed FEs</td>`
		`- <td className="bugscount">{this.props.data.beta_fe_proposed}</td>`
		`- </tr>`
		`- <tr>`
		`- <td>Accepted FEs</td>`
		`- <td className="bugscount">{this.props.data.beta_fe}</td>`
		`- </tr>`
		`- </tbody>`
		`- </table>`
		`- </div>`
		`+ const Blockers = (props) => {`
		`+ const [blockerbugs] = useState({});`
		`+ const [release] = useState(0);`

		`- <div className="col-md-6">`
		`- <table className="events">`
		`- <thead>`
		`- <tr>`
		`- <td colSpan="2">`
		`- <b>Final</b>`
		`- </td>`
		`- </tr>`
		`- </thead>`
		`- <tbody>`
		`- <tr>`
		`- <td>Proposed Blockers</td>`
		`- <td className="bugscount">{this.props.data.final_blockers_proposed}</td>`
		`- </tr>`
		`- <tr>`
		`- <td>Accepted Blockers</td>`
		`- <td className="bugscount">{this.props.data.final_blockers}</td>`
		`- </tr>`
		`- <tr>`
		`- <td>Proposed FEs</td>`
		`- <td className="bugscount">{this.props.data.final_fe_proposed}</td>`
		`- </tr>`
		`- <tr>`
		`- <td>Accepted FEs</td>`
		`- <td className="bugscount">{this.props.data.final_fe}</td>`
		`- </tr>`
		`- </tbody>`
		`- </table>`
		`- </div>`
		`- </Row>`
		`- </div>`
		`- )`
		`- }`
		`- }`
		`+ if (props.data)`
		`+ return (`
		`+ <div>`
		`+ <h1 className="padded">`
		`+ Fedora {props.release} blockers and FEs{" "}`
		`+ <SourceLink href="https://qa.fedoraproject.org/blockerbugs/" />`
		`+ </h1>`
		`+ <Row>`
		`+ <BugStats data={props.data} />`
		`+ </Row>`
		`+ </div>`
		`+ );`
		`+ };`

		`- export default Blockers`
		`+ export default Blockers;`

src/landingpage/BugStats.js

file added

+77

		`@@ -0,0 +1,77 @@`
		`+ import React from "react";`
		`+`
		`+ const BugStats = ({`
		`+ beta_blockers,`
		`+ beta_blockers_proposed,`
		`+ beta_fe,`
		`+ beta_fe_proposed,`
		`+ final_blockers_proposed,`
		`+ final_blockers,`
		`+ final_fe_proposed,`
		`+ final_fe`
		`+ }) => {`
		`+ return (`
		`+ <>`
		`+ <div className="col-md-6">`
		`+ <table className="events">`
		`+ <thead>`
		`+ <tr>`
		`+ <td colSpan="2">`
		`+ <b>Beta</b>`
		`+ </td>`
		`+ </tr>`
		`+ </thead>`
		`+ <tbody>`
		`+ <tr>`
		`+ <td>Proposed Blockers</td>`
		`+ <td className="bugscount">{beta_blockers_proposed}</td>`
		`+ </tr>`
		`+ <tr>`
		`+ <td>Accepted Blockers</td>`
		`+ <td className="bugscount">{beta_blockers}</td>`
		`+ </tr>`
		`+ <tr>`
		`+ <td>Proposed FEs</td>`
		`+ <td className="bugscount">{beta_fe_proposed}</td>`
		`+ </tr>`
		`+ <tr>`
		`+ <td>Accepted FEs</td>`
		`+ <td className="bugscount">{beta_fe}</td>`
		`+ </tr>`
		`+ </tbody>`
		`+ </table>`
		`+ </div>`
		`+`
		`+ <div className="col-md-6">`
		`+ <table className="events">`
		`+ <thead>`
		`+ <tr>`
		`+ <td colSpan="2">`
		`+ <b>Final</b>`
		`+ </td>`
		`+ </tr>`
		`+ </thead>`
		`+ <tbody>`
		`+ <tr>`
		`+ <td>Proposed Blockers</td>`
		`+ <td className="bugscount">{final_blockers_proposed}</td>`
		`+ </tr>`
		`+ <tr>`
		`+ <td>Accepted Blockers</td>`
		`+ <td className="bugscount">{final_blockers}</td>`
		`+ </tr>`
		`+ <tr>`
		`+ <td>Proposed FEs</td>`
		`+ <td className="bugscount">{final_fe_proposed}</td>`
		`+ </tr>`
		`+ <tr>`
		`+ <td>Accepted FEs</td>`
		`+ <td className="bugscount">{final_fe}</td>`
		`+ </tr>`
		`+ </tbody>`
		`+ </table>`
		`+ </div>`
		`+ </>`
		`+ );`
		`+ };`
		`+ export default BugStats;`

alisina92 commented 3 years ago

This change is related to this thread https://pagure.io/fedora-qa/landingpage/issue/6

My changes are:

/ I have created two functional Components called Beta Stats and Final Stat /
/ Moved the beta table to the final table to the corresponding functional component /
/ I have then passed the props.data into each Beta Stats and Final Stat Components /

optional changes:

/This wasn't part of the plan but I converted Blockers from class component to functional component also replaced the contractors with react Hooks as a developer is always good to be up to date with the tech stack /

manishakanyal commented 3 years ago

@alisina92, This is already on https://pagure.io/fedora-qa/landingpage/pull-request/15
And also, Moving the code to two files/components doesn't remove redundancy.

alisina92 commented 3 years ago

Dear manishakanyal,

Thank you for your reply, how can I help you with this component? I know
about the frontend but I am not familiar with a backend.
@lbrabec@redhat.com lbrabec@redhat.com told me that I can work on any
ticket regardless of who is doing it.

Kind Regards,

Alisina Bolourchi

On Thu, 1 Apr 2021 at 13:03, Manisha Kanyal pagure@pagure.io wrote:

manishakanyal commented on the pull-request: Redundancy removed from Blockers that you are following:
@alisina92, This is already on https://pagure.io/fedora-qa/landingpage/pull-request/15 And also, Moving the code to two files/components doesn't remove redundancy.

To reply, visit the link below or just reply to this email
https://pagure.io/fedora-qa/landingpage/pull-request/26

manishakanyal commented 3 years ago

no problem @alisina92.
But I don't see any message regarding this on the mailing list.
At most of the places, we do avoid working on something on which someone else is already working as it hinders the confidence of the one who's already working and also it makes do the same thing by multiple people.

alisina92 commented 3 years ago

Dear manishakanyal,
I know that we shouldn't work on the ticket that someone has picked up so
I asked @lbrabec@redhat.com lbrabec@redhat.com on the chat channel and
he said it wasn't said on the mailing list.

Kind Regards,

Alisina

On Thu, 1 Apr 2021 at 13:56, Manisha Kanyal pagure@pagure.io wrote:

manishakanyal commented on the pull-request: Redundancy removed from Blockers that you are following:
no problem @alisina92. But I don't see any message regarding this on the mailing list. At most of the places, we do avoid working on something on which someone else is already working as it hinders the confidence of the one who's already working and also it makes do the same thing by multiple people.

To reply, visit the link below or just reply to this email
https://pagure.io/fedora-qa/landingpage/pull-request/26

manishakanyal commented 3 years ago

@alisina92 No issues! I would say give loaders one issue a try.

lbrabec commented 3 years ago

This change didn't reduce the redundancy of code at all. Instead of having the same code twice in src/landingpage/Blockers.js, now there is the same code in two separate files src/landingpage/BetaStats.js and src/landingpage/FinalStats.js. One general component for both beta and final stats is a way to go here.

Metadata Update from @lbrabec:
- Pull-request tagged with: review in progress

3 years ago

alisina92 commented 3 years ago

Hi Brabec,

Thanks for your feedback I have another look.

Kind Regards,

Alisina

On Thu, 8 Apr 2021 at 10:20, Lukas Brabec pagure@pagure.io wrote:

lbrabec commented on the pull-request: Redundancy removed from Blockers
that you are following:
This change didn't reduce the redundancy of code at all. Instead of having the same code twice in `src/landingpage/Blockers.js`, now there is the same code in two separate files `src/landingpage/BetaStats.js` and `src/landingpage/FinalStats.js`. One general component for both beta and final stats is a way to go here.

To reply, visit the link below or just reply to this email
https://pagure.io/fedora-qa/landingpage/pull-request/26

1 new commit added

fixed the redundancy

3 years ago

alisina92 commented 3 years ago

Hi @lbrabec I have a new commit to be reviewed. thank you)

lbrabec commented 2 years ago

Thanks for participating in contribution period of Outreachy. This pull-request has not been selected to be merged and will be closed for housecleaning purposes. We may revisit this pull-request later, but as of now we will focus on merging pull-requests from our accepted intern.

Pull-Request has been closed by lbrabec

2 years ago