Skip to content

Comments

#BE-4777 netlify fixes#741

Merged
csonuryilmaz merged 4 commits intodevelopfrom
feature/be-4777
Nov 29, 2024
Merged

#BE-4777 netlify fixes#741
csonuryilmaz merged 4 commits intodevelopfrom
feature/be-4777

Conversation

@SarpBakis23
Copy link
Contributor

No description provided.

@SarpBakis23 SarpBakis23 added the Draft Draft PR label Nov 20, 2024
@SarpBakis23 SarpBakis23 self-assigned this Nov 20, 2024
@netlify
Copy link

netlify bot commented Nov 20, 2024

Deploy Preview for reverent-galileo-8ef035 ready!

Name Link
🔨 Latest commit 9105124
🔍 Latest deploy log https://app.netlify.com/sites/reverent-galileo-8ef035/deploys/67497feb051b7e00082525f2
😎 Deploy Preview https://deploy-preview-741--reverent-galileo-8ef035.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@SarpBakis23 SarpBakis23 removed the Draft Draft PR label Nov 21, 2024
Copy link
Contributor

@csonuryilmaz csonuryilmaz left a comment

Choose a reason for hiding this comment

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

File has some duplicate from URLs. There should be only one redirection for a path.

   2 from = "/integrations/working-with-custom-scripts/"
   2 from = "/workflows/android-specific-workflow-steps/build-and-test/android-build"
   2 from = "/workflows/common-workflow-steps/build-and-test/publish-release-notes"
   2 from = "/workflows/ios-specific-workflow-steps/distribution/browserstack-app-automation"

Copy link
Contributor

@csonuryilmaz csonuryilmaz left a comment

Choose a reason for hiding this comment

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

🧪 Tests

  • ✅ There should be no force option usage in redirects.
  • ✅ All redirects should have 301 redirected status code.
  • ✅ There should be no duplicate redirect from in the list.
  • ✅ All redirects should end with 200 and to page as defined.
  • ✅ There should be no circular redirect. (from == to case)

Disclaimer

The code review and test done in this PR checks all redirects are working as defined in the Netlify configuration file, and they're correctly defined as in Netlify documents. But it does not include any semantic checks.

For instance, if the Px is redirected to the page Py but it should have been redirected to Pz, this case is not included. When Py is an existing page and the user enters Px in the browser, the user is redirected to Py; it's 🆗 in the tests.

Reason (or assumption) is that all redirects were checked atomically in the context when they were merged first time in the previous PRs, so the redirects should have been correct semantically.

@github-actions github-actions bot added the autoupdate Useful when you want to autoupdate your branch when develop branch is updated label Nov 29, 2024
@csonuryilmaz csonuryilmaz removed the autoupdate Useful when you want to autoupdate your branch when develop branch is updated label Nov 29, 2024
@csonuryilmaz csonuryilmaz merged commit 8dc01f8 into develop Nov 29, 2024
@csonuryilmaz csonuryilmaz deleted the feature/be-4777 branch November 29, 2024 09:30
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.

2 participants