feat: rename trafficMesh flag to edgeHandlers #1906
Conversation
|
I have this as |
|
Thanks @tiffanosaurus! cc/ @erezrokah I believe this is okay if |
There was a problem hiding this comment.
Thanks @tiffanosaurus is there any issue we can discuss this?
To my knowledge the trafficMesh flag does more than add Edge Handlers support (which allows running custom logic at the edge).
Should we should call it edge or local-edge?
Also, could we keep the old flag and add a warning message to users instructing them to switch to the new flag so they can self serve?
We can drop the old flag completely after a couple of weeks with the new one.
As for the PR type:
chore:is a change that isn't visible to users (doesn't even require a new release)fix:is a bug fix (suggests a patch version)feat:is a user facing change (suggests a minor version) - so I believe this should be afeat:
To declare a breaking change you can use feat!:, but this isn't once since:
- The flag is hidden (like you mentioned).
- This is considered Beta (again like you mentioned).
|
Thanks @erezrokah! The issue and discussion around how to handle the flag can be found here: https://github.com/netlify/traffic-mesh/issues/1517 We can definitively add a new flag instead, we figured since it was beta, we could be more aggressive with the approach and replace it. |
tiffafoo
changed the title
github-actions
bot
added
the
type: feature
src/commands/dev/index.js
Outdated
| char: 't', | ||
| hidden: true, | ||
| description: 'uses Traffic Mesh for proxying requests', | ||
| description: '(DEPRECATED: use --edgeHandlers or -eh instead) uses Traffic Mesh for proxying requests', |
There was a problem hiding this comment.
Same here for the copy
src/commands/dev/index.js
Outdated
|
|
||
| if (flags.trafficMesh) { | ||
| warn( | ||
| '--trafficMesh and -t are deprecated and will be removed in the near future. Please use --edgeHandlers or -eh instead.', |
There was a problem hiding this comment.
Let me know if a different copy here would be better!
|
Updated the PR with the new flag and a deprecation notice for the This might have more changes as discussed in the issue since this not only enables edge handlers, but also the new redirect engine and people might want to enable it without wanting to use edge handlers. Would you prefer to put this in draft mode or iterate as we figure it out @erezrokah? |
src/commands/dev/index.js
Outdated
| description: 'start a public live session', | ||
| }), | ||
| edgeHandlers: flagsLib.boolean({ | ||
| char: 'eh', |
There was a problem hiding this comment.
it's common in the unix world to only have single-letter flags following a single-dash parameter so that you can combine multiple single letters in one param, e.g. tar -xzf which is short for tar -x -z -f. this kinda breaks with this convention and might leave users confused, wondering if there are isolated -e or -h (help?) flags?
multi-letter flags should always use double-dashes as a prefix imho
|
Yep of course and will update for the letter convention |
|
Heads up that the |
erezrokah
added
needs docs
and removed
type: bug
|
@tiffanosaurus when you merge this can you keep the commit message as |
tiffafoo
force-pushed
the
breaking/traffic-mesh-to-edge-handlers
branch
from
eb5634e to
be41c62
Compare
- Summary
The
--trafficMeshflag is not a super clear term to users, let's go for--edgeHandlerswhich is more to the point. Deprecate the current flag in favor of a. new flag and add a warning to users that use the old one.- Test plan
Choose your poison:
npm run test(follow Contributing.md for instructions)netlify dev --edgeHandlersnetlify dev -eh- Description for the changelog
Rename
netlify devflag--trafficMesh(short:-t) to--edgeHandlers(short:-eh) and update description toactivates the edge handlers runtime.- A picture of a cute animal (not mandatory but encouraged)
