Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

examples: remove app-dir and with prefix in the mdx directory name #73458

Open
wants to merge 1 commit into
base: canary
Choose a base branch
from

Conversation

JamBalaya56562
Copy link
Contributor

@JamBalaya56562 JamBalaya56562 commented Dec 3, 2024

What?

  1. Rename the following examples.
Before After
app-dir-mdx mdx
with-mdx mdx-pages
with-mdx-remote mdx-remote
  1. Change the example names at README.md in the repositories.
  2. Update the mdx pathname in the test files.

Why?

Why remove app-dir- in the directory name?

The default has already been App Router anyways and a lot of examples have migrated over.
x-ref: #72642 (comment)

Why remove with- in the directory name?

According to examples guidelines,

Example directories should not be prefixed with with-

How did you verify your code works?

I have tested the new with-mdx.test.ts and example.test.ts.

Adding or Updating Examples

CC: @samcx

@ijjk ijjk added examples Issue was opened via the examples template. tests labels Dec 3, 2024
@ijjk
Copy link
Member

ijjk commented Dec 3, 2024

Allow CI Workflow Run

  • approve CI run for commit: 097ed31

Note: this should only be enabled once the PR is ready to go and can only be enabled by a maintainer

@JamBalaya56562 JamBalaya56562 force-pushed the mdx branch 7 times, most recently from d8c24fb to 679be62 Compare December 10, 2024 23:51
@JamBalaya56562 JamBalaya56562 force-pushed the mdx branch 4 times, most recently from b1350ef to 4504933 Compare December 16, 2024 01:50
@samcx samcx self-requested a review December 17, 2024 01:42
@samcx
Copy link
Member

samcx commented Dec 17, 2024

@JamBalaya56562 It looks like there are conflicts now!

@JamBalaya56562 JamBalaya56562 force-pushed the mdx branch 2 times, most recently from 45885e7 to e2983ae Compare December 17, 2024 02:00
@JamBalaya56562
Copy link
Contributor Author

@JamBalaya56562 It looks like there are conflicts now!

Thank you for checking!
I have rebased and resolved merge conflict.

@samcx
Copy link
Member

samcx commented Dec 17, 2024

@JamBalaya56562 we should update mdx-remote to App Router as well!

@JamBalaya56562
Copy link
Contributor Author

@JamBalaya56562 we should update mdx-remote to App Router as well!

Alright, I have made PR of that at #74067.
Those changes are so big that I separated the PR.

This PR will be updated when #74067 is merged.

@samcx
Copy link
Member

samcx commented Dec 18, 2024

@JamBalaya56562 The other PR has been merged!

@JamBalaya56562
Copy link
Contributor Author

@JamBalaya56562 The other PR has been merged!

Thank you very much!
I have updated this PR, and there are no merge conflicts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
examples Issue was opened via the examples template. tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants