feat: implementa case-transformer server e documentação completa#1084
feat: implementa case-transformer server e documentação completa#1084glauccoeng-prog wants to merge 1 commit intomate-academy:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Implements the Node.js HTTP server for the “case-transformer” exercise, wiring URL parsing + validation to the existing convertToCase business logic, and updates project documentation/tooling.
Changes:
- Implemented
createServer()to parsereq.url, validatetextToConvert/toCase, and return JSON responses. - Refined README formatting around requirements/validation/response examples.
- Updated dev tooling dependency (
@mate-academy/scripts) and added a GitHub Actions workflow template.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/createServer.js |
Adds the HTTP server implementation, validations, and JSON responses delegating conversion to convertToCase. |
README.md |
Minor documentation formatting adjustments (spacing/JSON example formatting). |
package.json |
Bumps @mate-academy/scripts version. |
package-lock.json |
Lockfile updates corresponding to the scripts dependency bump. |
.github/workflows/test.yml-template |
Adds a CI workflow template for running tests on PRs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // A resposta deve ser sempre retornada em formato JSON | ||
| res.setHeader('Content-Type', 'application/json'); | ||
|
|
||
| // Separa a URL em duas partes: o caminho da rota principal | ||
| // (pathname) e os eventuais parâmetros (queryString) | ||
| const [pathname, queryString] = req.url.split('?'); | ||
| // Remove o caracter '/' inicial para isolar apenas o texto | ||
| // que a api vai converter | ||
| const textToConvert = pathname.slice(1); | ||
|
|
||
| // Usa a classe URLSearchParams (nativa do node) para processar | ||
| // os parâmetros enviados de forma fácil | ||
| const params = new URLSearchParams(queryString || ''); |
There was a problem hiding this comment.
The inline comments in this file are written in Portuguese, while the rest of the repository’s source/docs are in English (e.g., JSDoc in src/convertToCase/* and README). For consistency and easier maintenance across the team, please translate these comments to English (or remove non-essential comments).
| steps: | ||
| - uses: actions/checkout@v2 | ||
| - name: Use Node.js ${{ matrix.node-version }} | ||
| uses: actions/setup-node@v1 | ||
| with: | ||
| node-version: ${{ matrix.node-version }} | ||
| - run: npm install | ||
| - run: npm test |
There was a problem hiding this comment.
The added workflow YAML is invalidly indented: under steps: the list items must be indented further (e.g., steps:\n - uses: ...). As written, GitHub Actions will fail to parse it. Also, consider updating to supported action versions (checkout@v4, setup-node@v4) and aligning the branch filter with the repo default (this repo appears to use main, not master).
| "devDependencies": { | ||
| "@mate-academy/eslint-config": "latest", | ||
| "@mate-academy/scripts": "^1.8.6", | ||
| "@mate-academy/scripts": "^2.1.3", | ||
| "eslint": "^8.57.0", | ||
| "eslint-plugin-jest": "^28.6.0", |
There was a problem hiding this comment.
This PR’s title/goal is implementing the server and documentation, but it also bumps @mate-academy/scripts from ^1.8.6 to ^2.1.3. That upgrade can change lint/test behavior and Node version requirements (package-lock shows deps with node >= 18). If this upgrade isn’t required for the feature, please revert it (and the corresponding package-lock changes) to keep the PR focused; otherwise, document the reason in the PR description.
raulriato
left a comment
raulriato
left a comment
There was a problem hiding this comment.
Your code looks fine. Just a tip for next projects:
It's not good practice to leave comments on the final code.
No description provided.