DEV Community

ElshadHu
ElshadHu

Posted on

Contributing to Tableau-MCP

How I Chose the Repository

As I mentioned in my previous blog, I am interested in understanding MCP and how it works. I chose tableau-mcp, I chose this issue to work on. I successfully made a Pull Request that got merged. I plan to continue working on this project and contribute to it regularly.

What Was an Issue

REST API logging was missing the /api/3.26 path segment in logged URLs.I modified logRequest() and logResponse() functions to properly preserve the API version path when constructing URLs for logging.

After my changes I tested it with unit tests and the command below:

node -e "
const baseUrlObj = new URL('https://10ax.online.tableau.com/api/3.26');
const url = new URL(baseUrlObj.origin + baseUrlObj.pathname + '/sites/xxx/datasources');
console.log(url.toString());
"
Enter fullscreen mode Exit fullscreen mode

After confirming the fix worked correctly, I made the Pull Request. However, I forgot to bump the version, which I later committed using npm run version:patch. This is essential because it updates package.json and package-lock.json, keeping other developers aware of version changes.

Suggestions from Maintainer

When I initially wrote the changes, I used the URL constructor to be explicit about types:

const baseUrlObj = new URL(request.baseUrl);
  const url = maskedRequest.url?.startsWith('/')
    ? new URL(baseUrlObj.origin + baseUrlObj.pathname + maskedRequest.url)
    : new URL(maskedRequest.url ?? '', request.baseUrl);
Enter fullscreen mode Exit fullscreen mode

While this was explicit and clean code, the maintainer asked for my opinion on changing it to use the replace method for simplicity. That was reasonable because, APIs can change in the future unpredictably, and a simpler approach would be more maintainable. The suggestion made sense, so I changed it to this format:

 const url = new URL(
    `${maskedRequest.baseUrl.replace(/\/$/, '')}/${maskedRequest.url?.replace(/^\//, '') ?? ''}`,
  );
Enter fullscreen mode Exit fullscreen mode

As you can see, the code is simpler and most of the time in open source simple code is better than thinking about clean code. I applied the same pattern to logResponse() as well.

Gained Knowledge

As a result of this PR, I learned that it's better to discuss ideas with maintainers beforehand to get their insights. Talking to experienced developers provides more valuable learning than any tutorial or book.

Moving forward, I will:

  • Stop switching between projects and making only small contributions
  • Deepen my knowledge of the projects I've already discovered

I've already explored many projects, and I don't want to spend too much time setting up new ones and getting familiar with them. Instead, I'll contribute to what I've already discovered.

Top comments (0)