You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2022/01/27 14:40:52 UTC
[GitHub] [superset] geido opened a new pull request #18201: fix: Build scripts
geido opened a new pull request #18201:
URL: https://github.com/apache/superset/pull/18201
### SUMMARY
Fixes broken links and redirections to make the build scripts succeed.
### AFTER
<img width="1218" alt="Screen Shot 2022-01-27 at 15 39 28" src="https://user-images.githubusercontent.com/60598000/151380778-91ba6698-819c-4f38-b076-cdb9ed70e277.png">
### TESTING INSTRUCTIONS
1. Launch `npm run build`
2. Make sure it succeeds
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] etr2460 commented on a change in pull request #18201: fix: Build scripts
Posted by GitBox <gi...@apache.org>.
etr2460 commented on a change in pull request #18201:
URL: https://github.com/apache/superset/pull/18201#discussion_r793800616
##########
File path: docs-v2/docusaurus.config.js
##########
@@ -51,11 +51,11 @@ const config = {
from: '/tutorials.html',
},
{
- to: '/docs/creating-charts-dashboards/first-dashboard',
+ to: '/docs/Creating Charts and Dashboards/creating-your-first-dashboard',
Review comment:
not 100% sure what the change is here, but it's kinda odd to have spaces in a url. If this is replacing the old docs, then we should also make sure that redirects are setup between old and new urls
##########
File path: docs-v2/docusaurus.config.js
##########
@@ -30,7 +30,7 @@ const config = {
'Apache Superset is a modern data exploration and visualization platform',
url: 'https://superset.apache.org',
baseUrl: '/',
- onBrokenLinks: 'throw',
Review comment:
should we change this? throwing if we build docs with broken links seems reasonable to me
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] geido commented on a change in pull request #18201: fix: Build scripts
Posted by GitBox <gi...@apache.org>.
geido commented on a change in pull request #18201:
URL: https://github.com/apache/superset/pull/18201#discussion_r794473281
##########
File path: docs-v2/docusaurus.config.js
##########
@@ -51,11 +51,11 @@ const config = {
from: '/tutorials.html',
},
{
- to: '/docs/creating-charts-dashboards/first-dashboard',
+ to: '/docs/Creating Charts and Dashboards/creating-your-first-dashboard',
Review comment:
I agree that it's weird to have space in the URL but that's where the new page is sitting currently.
Not sure what do you mean about this though
> then we should also make sure that redirects are setup between old and new urls.
This is the scope of the redirect plugin. I am just making sure the links that it redirects to are correct, which they weren't in some cases.
@srinify would you like to double check?
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] geido commented on a change in pull request #18201: fix: Build scripts
Posted by GitBox <gi...@apache.org>.
geido commented on a change in pull request #18201:
URL: https://github.com/apache/superset/pull/18201#discussion_r794473281
##########
File path: docs-v2/docusaurus.config.js
##########
@@ -51,11 +51,11 @@ const config = {
from: '/tutorials.html',
},
{
- to: '/docs/creating-charts-dashboards/first-dashboard',
+ to: '/docs/Creating Charts and Dashboards/creating-your-first-dashboard',
Review comment:
I agree that it's weird to have spaces in the URL but that's where the new page is sitting currently.
Not sure what do you mean about this though
> then we should also make sure that redirects are setup between old and new urls.
This is the scope of the redirect plugin. I am just making sure the links that it redirects to are correct, which they weren't in some cases.
@srinify would you like to double-check?
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] geido commented on a change in pull request #18201: fix: Build scripts
Posted by GitBox <gi...@apache.org>.
geido commented on a change in pull request #18201:
URL: https://github.com/apache/superset/pull/18201#discussion_r795674044
##########
File path: docs-v2/docusaurus.config.js
##########
@@ -51,11 +51,11 @@ const config = {
from: '/tutorials.html',
},
{
- to: '/docs/creating-charts-dashboards/first-dashboard',
+ to: '/docs/Creating Charts and Dashboards/creating-your-first-dashboard',
Review comment:
Let's merge this PR which is unrelated to the way the URLs got created. We can discuss this separately and refactor the URLs in a separate PR.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] villebro commented on a change in pull request #18201: fix: Build scripts
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #18201:
URL: https://github.com/apache/superset/pull/18201#discussion_r795669105
##########
File path: docs-v2/docusaurus.config.js
##########
@@ -51,11 +51,11 @@ const config = {
from: '/tutorials.html',
},
{
- to: '/docs/creating-charts-dashboards/first-dashboard',
+ to: '/docs/Creating Charts and Dashboards/creating-your-first-dashboard',
Review comment:
I agree that having spaces in the path is funny. I think it would be preferable to rename the paths so that they are kebab or some other non-space notation, but maybe that can be left to a follow up?
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] geido commented on a change in pull request #18201: fix: Build scripts
Posted by GitBox <gi...@apache.org>.
geido commented on a change in pull request #18201:
URL: https://github.com/apache/superset/pull/18201#discussion_r794473281
##########
File path: docs-v2/docusaurus.config.js
##########
@@ -51,11 +51,11 @@ const config = {
from: '/tutorials.html',
},
{
- to: '/docs/creating-charts-dashboards/first-dashboard',
+ to: '/docs/Creating Charts and Dashboards/creating-your-first-dashboard',
Review comment:
I agree that it's weird to have space in the URL but that's where the new page is currently sitting. Not sure what do you mean about this though > then we should also make sure that redirects are setup between old and new urls. This is the scope of the redirect plugin. Here I am just making sure the links that it redirects to are correct, which they weren't in some cases.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] geido commented on a change in pull request #18201: fix: Build scripts
Posted by GitBox <gi...@apache.org>.
geido commented on a change in pull request #18201:
URL: https://github.com/apache/superset/pull/18201#discussion_r794472362
##########
File path: docs-v2/docusaurus.config.js
##########
@@ -30,7 +30,7 @@ const config = {
'Apache Superset is a modern data exploration and visualization platform',
url: 'https://superset.apache.org',
baseUrl: '/',
- onBrokenLinks: 'throw',
Review comment:
We can change it back when all the other pending PRs are merged as some links have been removed there. I'll take care of that
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] geido merged pull request #18201: fix: Build scripts
Posted by GitBox <gi...@apache.org>.
geido merged pull request #18201:
URL: https://github.com/apache/superset/pull/18201
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org