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

3400: Changed variable name from measurementID to measureName #3401

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

Conversation

RohitGavale
Copy link

Renamed the variable name from measurementID to measureName in dropMeasurementSeries Method. Changed the @PathVariable annotation to use measureName instead of measurementID.

@github-actions github-actions bot added java Pull requests that update Java code backend Everything that is related to the StreamPipes backend labels Jan 5, 2025
@RohitGavale RohitGavale changed the title 3400: Change variable name from measurementID to measureName 3400: Changed variable name from measurementID to measureName Jan 5, 2025
@tenthe
Copy link
Contributor

tenthe commented Jan 7, 2025

Hi @RohitGavale,
Thank you very much for your PR, we really appreciate it.

For this PR, the clients that call the REST interface also need to be updated, can you please adjust this?
If you have any questions or problems, just write us. We are happy to help you.

@RohitGavale
Copy link
Author

Hi @RohitGavale, Thank you very much for your PR, we really appreciate it.

For this PR, the clients that call the REST interface also need to be updated, can you please adjust this? If you have any questions or problems, just write us. We are happy to help you.

Could you confirm if the expectation is to update measurementID to measurementName in all client calls to align with the changes made in the REST interface?

@tenthe
Copy link
Contributor

tenthe commented Jan 7, 2025

Yes, exactly.

@RohitGavale
Copy link
Author

I have made a pull request.

@tenthe
Copy link
Contributor

tenthe commented Jan 8, 2025

Hi @RohitGavale,

Thank you for updating your PR in #3402! I really appreciate your effort.

To clarify, there’s no need to open a new PR for the requested changes—you can make the updates directly in this one.

My earlier comment might have been unclear, sorry for that. I’d like to refine it: I didn’t mean for you to modify the other methods within the DataLakeResourceV4 class. Instead, I was referring specifically to the TypeScript client that interacts with this REST interface. Could you please update the naming in the datalake-rest.service.ts file, particularly for the dropSingleMeasurementSeries method?

Here’s my suggestion:

  • Let’s focus on addressing the required updates in this PR, keeping the scope manageable.
  • Changes to the naming of other interfaces can be handled in a separate issue or PR.

Additionally, to resolve the failing tests for this PR, you’ll need to adapt the variable name in the @DeleteMapping annotation, as you already did in your other PR.

I hope this comment is helpful.
Thank you again for your work and contributions.

Cheers,
Philipp

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend Everything that is related to the StreamPipes backend java Pull requests that update Java code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants