Skip to content

Conversation

@scbedd
Copy link
Member

@scbedd scbedd commented Jan 25, 2022

@sima-zhu I was investigating Azure/azure-sdk-for-python#16794 and discovered this.

In the case where there is a "/" in the name it gets used directly! This means on a browser that the link to the landing page won't ever render because a / means "next folder in" on a webserver!

So for the service Database for MySQL/PostGreSQL, this turns into path databaseformysql/postgresql.html. This is 404ing.

@azure-sdk
Copy link
Collaborator

The following pipelines have been queued for testing:
java - template
java - template - tests
js - template
net - template
net - template - tests
python - template
python - template - tests
You can sign off on the approval gate to test the release stage of each pipeline.
See eng/common workflow

$fileName = ($serviceName -replace '\s', '').ToLower().Trim()
# handle spaces in service name, EG "Confidential Ledger"
# handle / in service name, EG "Database for MySQL/PostgreSQL". Leaving a "/" present will generate a bad link location.
$fileName = ($serviceName -replace '\s', '').Replace("/","").ToLower().Trim()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@danieljurek it looks like we might want to start having some helper for this as we are doing this in a few places.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we put check into one regex?

$serviceName -replace "\s|\\|\/, ""

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had already merged this in a few repos before this feedback came in. I'm going to merge this PR.

@ghost
Copy link

ghost commented Jan 26, 2022

Hello @azure-sdk!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@scbedd
Copy link
Member Author

scbedd commented Jan 26, 2022

/check-enforcer override

@ghost ghost merged commit 8cca9a4 into Azure:main Jan 26, 2022
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants