You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by GitBox <gi...@apache.org> on 2022/02/13 15:04:53 UTC
[GitHub] [qpid-dispatch] jiridanek opened a new pull request #1517: DISPATCH-2234: Update JavaScript console packages for the 1.19.0 release
jiridanek opened a new pull request #1517:
URL: https://github.com/apache/qpid-dispatch/pull/1517
* Error: A <Route> is only ever to be used as the child of <Routes> element, never rendered directly. Please wrap your <Route> in a <Routes>.
* Error: You cannot render a <Router> inside another <Router>. You should never have more than one in your app.
See https://reactrouter.com/docs/en/v6/upgrading/v5, and therein linked https://gist.github.com/mjackson/d54b40a094277b7afdd6b81f51a0393f
* BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default.
* Module not found: Error: Can't resolve 'os' in '/home/jdanek/repos/qpid/qpid-dispatch/console/react/node_modules/rhea/lib'
* https://github.com/facebook/create-react-app/issues/11756
* filled https://github.com/amqp/rhea/issues/379
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-dispatch] codecov-commenter commented on pull request #1517: DISPATCH-2234: Update JavaScript console packages for the 1.19.0 release
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #1517:
URL: https://github.com/apache/qpid-dispatch/pull/1517#issuecomment-1038198028
# [Codecov](https://codecov.io/gh/apache/qpid-dispatch/pull/1517?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#1517](https://codecov.io/gh/apache/qpid-dispatch/pull/1517?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (df91ed9) into [main](https://codecov.io/gh/apache/qpid-dispatch/commit/542fa4035b9cd9cb74d448d518e388d2a18f7d68?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (542fa40) will **decrease** coverage by `0.03%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/qpid-dispatch/pull/1517?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## main #1517 +/- ##
==========================================
- Coverage 84.92% 84.89% -0.04%
==========================================
Files 116 117 +1
Lines 28640 28642 +2
==========================================
- Hits 24323 24315 -8
- Misses 4317 4327 +10
```
| [Impacted Files](https://codecov.io/gh/apache/qpid-dispatch/pull/1517?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...router\_core/modules/edge\_router/link\_route\_proxy.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1517/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9jb3JlL21vZHVsZXMvZWRnZV9yb3V0ZXIvbGlua19yb3V0ZV9wcm94eS5j) | `78.69% <0.00%> (-4.15%)` | :arrow_down: |
| [src/router\_core/agent\_address.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1517/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9jb3JlL2FnZW50X2FkZHJlc3MuYw==) | `88.18% <0.00%> (-1.58%)` | :arrow_down: |
| [src/router\_core/modules/edge\_router/edge\_mgmt.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1517/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9jb3JlL21vZHVsZXMvZWRnZV9yb3V0ZXIvZWRnZV9tZ210LmM=) | `84.15% <0.00%> (-1.00%)` | :arrow_down: |
| [src/adaptors/tcp\_adaptor.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1517/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL2FkYXB0b3JzL3RjcF9hZGFwdG9yLmM=) | `77.10% <0.00%> (-0.32%)` | :arrow_down: |
| [src/router\_node.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1517/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9ub2RlLmM=) | `93.80% <0.00%> (-0.20%)` | :arrow_down: |
| [src/router\_core/connections.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1517/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9jb3JlL2Nvbm5lY3Rpb25zLmM=) | `90.14% <0.00%> (ø)` | |
| [src/router\_core/router\_core\_private.h](https://codecov.io/gh/apache/qpid-dispatch/pull/1517/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9jb3JlL3JvdXRlcl9jb3JlX3ByaXZhdGUuaA==) | `100.00% <0.00%> (ø)` | |
| [src/iterator.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1517/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL2l0ZXJhdG9yLmM=) | `89.94% <0.00%> (+0.16%)` | :arrow_up: |
| [src/router\_core/router\_core.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1517/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9jb3JlL3JvdXRlcl9jb3JlLmM=) | `87.22% <0.00%> (+0.17%)` | :arrow_up: |
| [src/router\_core/transfer.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1517/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9jb3JlL3RyYW5zZmVyLmM=) | `94.50% <0.00%> (+0.63%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/qpid-dispatch/pull/1517?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/qpid-dispatch/pull/1517?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [542fa40...df91ed9](https://codecov.io/gh/apache/qpid-dispatch/pull/1517?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-dispatch] bartoval commented on pull request #1517: DISPATCH-2234: Update JavaScript console packages for the 1.19.0 release
Posted by GitBox <gi...@apache.org>.
bartoval commented on pull request #1517:
URL: https://github.com/apache/qpid-dispatch/pull/1517#issuecomment-1041872307
Great job +1 !
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-dispatch] jiridanek commented on a change in pull request #1517: DISPATCH-2234: Update JavaScript console packages for the 1.19.0 release
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #1517:
URL: https://github.com/apache/qpid-dispatch/pull/1517#discussion_r807979965
##########
File path: console/react/src/App.js
##########
@@ -28,26 +29,22 @@ import "@patternfly/patternfly/components/Nav/nav.css";
import { QDRService } from "./common/qdrService";
import "./App.css";
import PageLayout from "./overview/dashboard/layout";
-class App extends Component {
- state = {};
-
- render() {
- // service is passed in to make testing easier
- const service = new QDRService();
- // also, a router is used here to provide PageLayout with a history property
- return (
- <Router>
- <div className="App pf-m-redhat-font">
- <Route
- path="/"
- render={props => (
- <PageLayout service={service} {...props} config={this.props.config} />
- )}
- />
- </div>
- </Router>
- );
- }
+
+function App(props) {
+
+ // service is passed in to make testing easier
+ let service = new QDRService();
+
+ // also, a router is used here to provide PageLayout with a history property
+ let history = createHashHistory({ window });
+
+ return (
+ <HistoryRouter history={history}>
Review comment:
Yeah, that is how you are supposed to use HistoryRouter, according to docs, https://reactrouter.com/docs/en/v6/api#unstable_historyrouter
Honestly, there are two things in the console, related to react-router-dom, that I think can be now done better
- navigating between pages; which is now done inconsistently, either with `<Navigate>`, or `navigate()`, or through `history`.
- the page structure which is supposed to use the `<Outlet>` feature but it does not
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-dispatch] bartoval commented on a change in pull request #1517: DISPATCH-2234: Update JavaScript console packages for the 1.19.0 release
Posted by GitBox <gi...@apache.org>.
bartoval commented on a change in pull request #1517:
URL: https://github.com/apache/qpid-dispatch/pull/1517#discussion_r807832047
##########
File path: console/react/src/App.js
##########
@@ -28,26 +29,22 @@ import "@patternfly/patternfly/components/Nav/nav.css";
import { QDRService } from "./common/qdrService";
import "./App.css";
import PageLayout from "./overview/dashboard/layout";
-class App extends Component {
- state = {};
-
- render() {
- // service is passed in to make testing easier
- const service = new QDRService();
- // also, a router is used here to provide PageLayout with a history property
- return (
- <Router>
- <div className="App pf-m-redhat-font">
- <Route
- path="/"
- render={props => (
- <PageLayout service={service} {...props} config={this.props.config} />
- )}
- />
- </div>
- </Router>
- );
- }
+
+function App(props) {
+
+ // service is passed in to make testing easier
+ let service = new QDRService();
Review comment:
You can safetely use the _const_ keyword instead of _let_ here and in line 39.
`let service = new QDRService();`
##########
File path: console/react/src/App.js
##########
@@ -28,26 +29,22 @@ import "@patternfly/patternfly/components/Nav/nav.css";
import { QDRService } from "./common/qdrService";
import "./App.css";
import PageLayout from "./overview/dashboard/layout";
-class App extends Component {
- state = {};
-
- render() {
- // service is passed in to make testing easier
- const service = new QDRService();
- // also, a router is used here to provide PageLayout with a history property
- return (
- <Router>
- <div className="App pf-m-redhat-font">
- <Route
- path="/"
- render={props => (
- <PageLayout service={service} {...props} config={this.props.config} />
- )}
- />
- </div>
- </Router>
- );
- }
+
+function App(props) {
+
+ // service is passed in to make testing easier
+ let service = new QDRService();
+
+ // also, a router is used here to provide PageLayout with a history property
+ let history = createHashHistory({ window });
+
+ return (
+ <HistoryRouter history={history}>
Review comment:
Why we are using HistoryRouter here? Before this refactoring, we used the HashRouter.
##########
File path: console/react/package.json
##########
@@ -1,25 +1,25 @@
{
"name": "dispatch-console",
- "version": "1.17.0",
+ "version": "1.19.0",
"private": true,
"dependencies": {
- "@patternfly/patternfly": "^4.125.3",
- "@patternfly/react-charts": "^6.15.14",
- "@patternfly/react-core": "^4.147.0",
- "@patternfly/react-icons": "^4.11.8",
- "@patternfly/react-styles": "^4.11.8",
- "@patternfly/react-table": "^4.29.37",
- "@patternfly/react-topology": "^4.9.42",
+ "@patternfly/patternfly": "^4.171.1",
+ "@patternfly/react-charts": "^6.45.15",
+ "@patternfly/react-core": "^4.192.15",
+ "@patternfly/react-icons": "^4.43.15",
+ "@patternfly/react-styles": "^4.42.15",
+ "@patternfly/react-table": "^4.61.15",
+ "@patternfly/react-topology": "^4.39.15",
"d3": "^3.5.17",
"d3-queue": "^3.0.7",
- "express": "^4.17.1",
+ "express": "^4.17.2",
"font-awesome": "^4.7.0",
- "prop-types": "^15.7.2",
+ "prop-types": "^15.8.1",
"react": "^17.0.2",
"react-dom": "^17.0.2",
"react-fontawesome": "^1.7.1",
- "react-router-dom": "^5.2.0",
- "rhea": "^2.0.4",
+ "react-router-dom": "^6.2.1",
Review comment:
That is a breaking change. is it worth changing the library?
##########
File path: console/react/src/overview/dashboard/layout.test.js
##########
@@ -21,6 +21,7 @@ import React from "react";
import { render } from "@testing-library/react";
import { service } from "../../serviceTest";
import PageLayout from "./layout";
+import {MemoryRouter} from "react-router-dom";
Review comment:
Why we are using MemoryRouter here?
I see in the code we use the History Router.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-dispatch] jiridanek commented on a change in pull request #1517: DISPATCH-2234: Update JavaScript console packages for the 1.19.0 release
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #1517:
URL: https://github.com/apache/qpid-dispatch/pull/1517#discussion_r807960231
##########
File path: console/react/src/App.js
##########
@@ -28,26 +29,22 @@ import "@patternfly/patternfly/components/Nav/nav.css";
import { QDRService } from "./common/qdrService";
import "./App.css";
import PageLayout from "./overview/dashboard/layout";
-class App extends Component {
- state = {};
-
- render() {
- // service is passed in to make testing easier
- const service = new QDRService();
- // also, a router is used here to provide PageLayout with a history property
- return (
- <Router>
- <div className="App pf-m-redhat-font">
- <Route
- path="/"
- render={props => (
- <PageLayout service={service} {...props} config={this.props.config} />
- )}
- />
- </div>
- </Router>
- );
- }
+
+function App(props) {
+
+ // service is passed in to make testing easier
+ let service = new QDRService();
+
+ // also, a router is used here to provide PageLayout with a history property
+ let history = createHashHistory({ window });
+
+ return (
+ <HistoryRouter history={history}>
Review comment:
Because the code of some components nested further in needs to have `history` instance in properties. That is not available in react-router-dom v6 unless I put it there. And `HistoryRouter` is the way to inject `history`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-dispatch] jiridanek commented on pull request #1517: DISPATCH-2234: Update JavaScript console packages for the 1.19.0 release
Posted by GitBox <gi...@apache.org>.
jiridanek commented on pull request #1517:
URL: https://github.com/apache/qpid-dispatch/pull/1517#issuecomment-1038191741
GHA failed on TSAN issue in Proton 0.36; that one should be already fixed on main.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-dispatch] jiridanek merged pull request #1517: DISPATCH-2234: Update JavaScript console packages for the 1.19.0 release
Posted by GitBox <gi...@apache.org>.
jiridanek merged pull request #1517:
URL: https://github.com/apache/qpid-dispatch/pull/1517
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-dispatch] jiridanek commented on a change in pull request #1517: DISPATCH-2234: Update JavaScript console packages for the 1.19.0 release
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #1517:
URL: https://github.com/apache/qpid-dispatch/pull/1517#discussion_r807959792
##########
File path: console/react/src/App.js
##########
@@ -28,26 +29,22 @@ import "@patternfly/patternfly/components/Nav/nav.css";
import { QDRService } from "./common/qdrService";
import "./App.css";
import PageLayout from "./overview/dashboard/layout";
-class App extends Component {
- state = {};
-
- render() {
- // service is passed in to make testing easier
- const service = new QDRService();
- // also, a router is used here to provide PageLayout with a history property
- return (
- <Router>
- <div className="App pf-m-redhat-font">
- <Route
- path="/"
- render={props => (
- <PageLayout service={service} {...props} config={this.props.config} />
- )}
- />
- </div>
- </Router>
- );
- }
+
+function App(props) {
+
+ // service is passed in to make testing easier
+ let service = new QDRService();
Review comment:
:+1:
##########
File path: console/react/src/overview/dashboard/layout.test.js
##########
@@ -21,6 +21,7 @@ import React from "react";
import { render } from "@testing-library/react";
import { service } from "../../serviceTest";
import PageLayout from "./layout";
+import {MemoryRouter} from "react-router-dom";
Review comment:
`MemoryRouter` is intended for testing in nodejs (outside of browser). `HistoryRouter` is meant to allow injecting your own `history` instance. (https://reactrouter.com/docs/en/v6/api#setup)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-dispatch] bartoval commented on a change in pull request #1517: DISPATCH-2234: Update JavaScript console packages for the 1.19.0 release
Posted by GitBox <gi...@apache.org>.
bartoval commented on a change in pull request #1517:
URL: https://github.com/apache/qpid-dispatch/pull/1517#discussion_r807832047
##########
File path: console/react/src/App.js
##########
@@ -28,26 +29,22 @@ import "@patternfly/patternfly/components/Nav/nav.css";
import { QDRService } from "./common/qdrService";
import "./App.css";
import PageLayout from "./overview/dashboard/layout";
-class App extends Component {
- state = {};
-
- render() {
- // service is passed in to make testing easier
- const service = new QDRService();
- // also, a router is used here to provide PageLayout with a history property
- return (
- <Router>
- <div className="App pf-m-redhat-font">
- <Route
- path="/"
- render={props => (
- <PageLayout service={service} {...props} config={this.props.config} />
- )}
- />
- </div>
- </Router>
- );
- }
+
+function App(props) {
+
+ // service is passed in to make testing easier
+ let service = new QDRService();
Review comment:
You can safetely use the _const_ keyword instead of _let_ here and in line 39.
`const service = new QDRService();`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-dispatch] jiridanek commented on a change in pull request #1517: DISPATCH-2234: Update JavaScript console packages for the 1.19.0 release
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #1517:
URL: https://github.com/apache/qpid-dispatch/pull/1517#discussion_r808559059
##########
File path: console/react/src/App.js
##########
@@ -28,26 +29,22 @@ import "@patternfly/patternfly/components/Nav/nav.css";
import { QDRService } from "./common/qdrService";
import "./App.css";
import PageLayout from "./overview/dashboard/layout";
-class App extends Component {
- state = {};
-
- render() {
- // service is passed in to make testing easier
- const service = new QDRService();
- // also, a router is used here to provide PageLayout with a history property
- return (
- <Router>
- <div className="App pf-m-redhat-font">
- <Route
- path="/"
- render={props => (
- <PageLayout service={service} {...props} config={this.props.config} />
- )}
- />
- </div>
- </Router>
- );
- }
+
+function App(props) {
+
+ // service is passed in to make testing easier
+ let service = new QDRService();
+
+ // also, a router is used here to provide PageLayout with a history property
+ let history = createHashHistory({ window });
+
+ return (
+ <HistoryRouter history={history}>
Review comment:
Noted in https://issues.apache.org/jira/browse/DISPATCH-2331
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-dispatch] codecov-commenter commented on pull request #1517: DISPATCH-2234: Update JavaScript console packages for the 1.19.0 release
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #1517:
URL: https://github.com/apache/qpid-dispatch/pull/1517#issuecomment-1038198028
# [Codecov](https://codecov.io/gh/apache/qpid-dispatch/pull/1517?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#1517](https://codecov.io/gh/apache/qpid-dispatch/pull/1517?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (df91ed9) into [main](https://codecov.io/gh/apache/qpid-dispatch/commit/542fa4035b9cd9cb74d448d518e388d2a18f7d68?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (542fa40) will **decrease** coverage by `0.03%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/qpid-dispatch/pull/1517?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## main #1517 +/- ##
==========================================
- Coverage 84.92% 84.89% -0.04%
==========================================
Files 116 117 +1
Lines 28640 28642 +2
==========================================
- Hits 24323 24315 -8
- Misses 4317 4327 +10
```
| [Impacted Files](https://codecov.io/gh/apache/qpid-dispatch/pull/1517?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...router\_core/modules/edge\_router/link\_route\_proxy.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1517/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9jb3JlL21vZHVsZXMvZWRnZV9yb3V0ZXIvbGlua19yb3V0ZV9wcm94eS5j) | `78.69% <0.00%> (-4.15%)` | :arrow_down: |
| [src/router\_core/agent\_address.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1517/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9jb3JlL2FnZW50X2FkZHJlc3MuYw==) | `88.18% <0.00%> (-1.58%)` | :arrow_down: |
| [src/router\_core/modules/edge\_router/edge\_mgmt.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1517/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9jb3JlL21vZHVsZXMvZWRnZV9yb3V0ZXIvZWRnZV9tZ210LmM=) | `84.15% <0.00%> (-1.00%)` | :arrow_down: |
| [src/adaptors/tcp\_adaptor.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1517/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL2FkYXB0b3JzL3RjcF9hZGFwdG9yLmM=) | `77.10% <0.00%> (-0.32%)` | :arrow_down: |
| [src/router\_node.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1517/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9ub2RlLmM=) | `93.80% <0.00%> (-0.20%)` | :arrow_down: |
| [src/router\_core/connections.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1517/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9jb3JlL2Nvbm5lY3Rpb25zLmM=) | `90.14% <0.00%> (ø)` | |
| [src/router\_core/router\_core\_private.h](https://codecov.io/gh/apache/qpid-dispatch/pull/1517/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9jb3JlL3JvdXRlcl9jb3JlX3ByaXZhdGUuaA==) | `100.00% <0.00%> (ø)` | |
| [src/iterator.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1517/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL2l0ZXJhdG9yLmM=) | `89.94% <0.00%> (+0.16%)` | :arrow_up: |
| [src/router\_core/router\_core.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1517/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9jb3JlL3JvdXRlcl9jb3JlLmM=) | `87.22% <0.00%> (+0.17%)` | :arrow_up: |
| [src/router\_core/transfer.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1517/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9jb3JlL3RyYW5zZmVyLmM=) | `94.50% <0.00%> (+0.63%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/qpid-dispatch/pull/1517?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/qpid-dispatch/pull/1517?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [542fa40...df91ed9](https://codecov.io/gh/apache/qpid-dispatch/pull/1517?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-dispatch] bartoval commented on a change in pull request #1517: DISPATCH-2234: Update JavaScript console packages for the 1.19.0 release
Posted by GitBox <gi...@apache.org>.
bartoval commented on a change in pull request #1517:
URL: https://github.com/apache/qpid-dispatch/pull/1517#discussion_r807967089
##########
File path: console/react/src/App.js
##########
@@ -28,26 +29,22 @@ import "@patternfly/patternfly/components/Nav/nav.css";
import { QDRService } from "./common/qdrService";
import "./App.css";
import PageLayout from "./overview/dashboard/layout";
-class App extends Component {
- state = {};
-
- render() {
- // service is passed in to make testing easier
- const service = new QDRService();
- // also, a router is used here to provide PageLayout with a history property
- return (
- <Router>
- <div className="App pf-m-redhat-font">
- <Route
- path="/"
- render={props => (
- <PageLayout service={service} {...props} config={this.props.config} />
- )}
- />
- </div>
- </Router>
- );
- }
+
+function App(props) {
+
+ // service is passed in to make testing easier
+ let service = new QDRService();
+
+ // also, a router is used here to provide PageLayout with a history property
+ let history = createHashHistory({ window });
+
+ return (
+ <HistoryRouter history={history}>
Review comment:
Here I see that the child component PageLayout pass explicitly the pro ```history={history}``` and history coming from ``` createHashHistory({ window });```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-dispatch] jiridanek commented on a change in pull request #1517: DISPATCH-2234: Update JavaScript console packages for the 1.19.0 release
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #1517:
URL: https://github.com/apache/qpid-dispatch/pull/1517#discussion_r807959553
##########
File path: console/react/package.json
##########
@@ -1,25 +1,25 @@
{
"name": "dispatch-console",
- "version": "1.17.0",
+ "version": "1.19.0",
"private": true,
"dependencies": {
- "@patternfly/patternfly": "^4.125.3",
- "@patternfly/react-charts": "^6.15.14",
- "@patternfly/react-core": "^4.147.0",
- "@patternfly/react-icons": "^4.11.8",
- "@patternfly/react-styles": "^4.11.8",
- "@patternfly/react-table": "^4.29.37",
- "@patternfly/react-topology": "^4.9.42",
+ "@patternfly/patternfly": "^4.171.1",
+ "@patternfly/react-charts": "^6.45.15",
+ "@patternfly/react-core": "^4.192.15",
+ "@patternfly/react-icons": "^4.43.15",
+ "@patternfly/react-styles": "^4.42.15",
+ "@patternfly/react-table": "^4.61.15",
+ "@patternfly/react-topology": "^4.39.15",
"d3": "^3.5.17",
"d3-queue": "^3.0.7",
- "express": "^4.17.1",
+ "express": "^4.17.2",
"font-awesome": "^4.7.0",
- "prop-types": "^15.7.2",
+ "prop-types": "^15.8.1",
"react": "^17.0.2",
"react-dom": "^17.0.2",
"react-fontawesome": "^1.7.1",
- "react-router-dom": "^5.2.0",
- "rhea": "^2.0.4",
+ "react-router-dom": "^6.2.1",
Review comment:
I think so, but there is not a thoughtfully developed policy on this in the dispatch-router project.
Normally, for the native code components (the qdrouterd itself), the versions of dependencies are dictated by the minimum version present in Linux distributions that needs to be supported; that is, centos/rhel/fedora and ubuntu. This is how C projects usually work. There are some packaging systems for C, similar to npm, but the traditional Linux practice is to rely on your distro.
In Javascript, all dependencies come from npm, so there the only limitation coming from distros is version of nodejs.
I believe that the most sensible policy is to keep up with the latest upstream version and to upgrade often. Here are my reasons for that
- security: minimizes the amount of `npm audit` warnings
- ease of update: when required (due to npm audit warnings), it helps if we are on still actively supported recent version
- small diffs on upgrade: when the upgrade is done soon, the amount of changes will be smaller than when having to upgrade over multiple major versions
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-dispatch] bartoval commented on a change in pull request #1517: DISPATCH-2234: Update JavaScript console packages for the 1.19.0 release
Posted by GitBox <gi...@apache.org>.
bartoval commented on a change in pull request #1517:
URL: https://github.com/apache/qpid-dispatch/pull/1517#discussion_r808227824
##########
File path: console/react/src/App.js
##########
@@ -28,26 +29,22 @@ import "@patternfly/patternfly/components/Nav/nav.css";
import { QDRService } from "./common/qdrService";
import "./App.css";
import PageLayout from "./overview/dashboard/layout";
-class App extends Component {
- state = {};
-
- render() {
- // service is passed in to make testing easier
- const service = new QDRService();
- // also, a router is used here to provide PageLayout with a history property
- return (
- <Router>
- <div className="App pf-m-redhat-font">
- <Route
- path="/"
- render={props => (
- <PageLayout service={service} {...props} config={this.props.config} />
- )}
- />
- </div>
- </Router>
- );
- }
+
+function App(props) {
+
+ // service is passed in to make testing easier
+ let service = new QDRService();
+
+ // also, a router is used here to provide PageLayout with a history property
+ let history = createHashHistory({ window });
+
+ return (
+ <HistoryRouter history={history}>
Review comment:
Good point +1.
Probably one of the things to do is refactor the PageLayout component as a function (but this is out of the scope of this PR honestly) to use directly hooks. So we can avoid passing history and use directly useNavigate and useLocation. Then probably we can avoid passing {...props } because without typescript we lose a bit the control of what we inject in the component. Moreover the High order component 'WithRoute' will be deleted
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-dispatch] jiridanek commented on pull request #1517: DISPATCH-2234: Update JavaScript console packages for the 1.19.0 release
Posted by GitBox <gi...@apache.org>.
jiridanek commented on pull request #1517:
URL: https://github.com/apache/qpid-dispatch/pull/1517#issuecomment-1038191741
GHA failed on TSAN issue in Proton 0.36; that one should be already fixed on main.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org