fix: detect server subfolder in init#566
fix: detect server subfolder in init#566domdomegg merged 4 commits intomodelcontextprotocol:mainfrom
Conversation
|
Thanks! Are you able to fix the merge conflicts and we can get this shipped 🚀 |
7a3e409 to
514c92d
Compare
|
awesome, glad to hear it @domdomegg! I rebased and fixed the merge conflicts :) |
rdimitrov
left a comment
There was a problem hiding this comment.
Thanks! Left a small comment 👍
cmd/publisher/commands/init.go
Outdated
| // If we're in a subdirectory, use the current folder name | ||
| if subfolder != "" { | ||
| if cwd, err := os.Getwd(); err == nil { | ||
| folderName := filepath.Base(cwd) |
There was a problem hiding this comment.
It seems we use subfolder as a boolean even though it's a string. Wouldn't it be better to reuse the subfolder variable we already have?
For example if it's != "" construct the path using it (and avoid reading the base path again), otherwise proceed as if we are in the root repo.
There was a problem hiding this comment.
you're completely right, I pushed a fix
|
Hey! Any update on this? Looks like I got an approval and fixed the comment from the other review so is it ready to be merged? |
<!-- Provide a brief summary of your changes --> ## Motivation and Context Discussion from this issue: modelcontextprotocol#538 We want to make it clearer that repo name & server name do not HAVE to be the same, especially in monorepos. @tadasant this seems like a minor thing that doesn't have to be documented specifically, lmk if you disagree :) ## How Has This Been Tested? Tested the init flow locally, running the built `init` command from a subfolder and the repo root. ## Breaking Changes No ## Types of changes <!-- What types of changes does your code introduce? Put an `x` in all the boxes that apply: --> - [ ] Bug fix (non-breaking change which fixes an issue) - [x] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] Documentation update ## Checklist <!-- Go over all the following points, and put an `x` in all the boxes that apply. --> - [x] I have read the [MCP Documentation](https://modelcontextprotocol.io) - [x] My code follows the repository's style guidelines - [x] New and existing tests pass locally - [x] I have added appropriate error handling - [x] I have added or updated documentation as needed ## Additional context <!-- Add any other context, implementation notes, or design decisions -->
<!-- Provide a brief summary of your changes --> ## Motivation and Context Discussion from this issue: modelcontextprotocol#538 We want to make it clearer that repo name & server name do not HAVE to be the same, especially in monorepos. @tadasant this seems like a minor thing that doesn't have to be documented specifically, lmk if you disagree :) ## How Has This Been Tested? Tested the init flow locally, running the built `init` command from a subfolder and the repo root. ## Breaking Changes No ## Types of changes <!-- What types of changes does your code introduce? Put an `x` in all the boxes that apply: --> - [ ] Bug fix (non-breaking change which fixes an issue) - [x] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] Documentation update ## Checklist <!-- Go over all the following points, and put an `x` in all the boxes that apply. --> - [x] I have read the [MCP Documentation](https://modelcontextprotocol.io) - [x] My code follows the repository's style guidelines - [x] New and existing tests pass locally - [x] I have added appropriate error handling - [x] I have added or updated documentation as needed ## Additional context <!-- Add any other context, implementation notes, or design decisions -->
Motivation and Context
Discussion from this issue: #538
We want to make it clearer that repo name & server name do not HAVE to be the same, especially in monorepos.
@tadasant this seems like a minor thing that doesn't have to be documented specifically, lmk if you disagree :)
How Has This Been Tested?
Tested the init flow locally, running the built
initcommand from a subfolder and the repo root.Breaking Changes
No
Types of changes
Checklist
Additional context