chore: Override arguments in all services inheriting from templates

[pbmiguel]

Problem - Code Readability

• When using MergeRequestNotes all method it's not explicit what its parameters consist. The only thing we know is that it receives two parameters -> resourceId; and resource2Id.
  (just as depicted in the image bellow)
  

• So here lies the problem, what really is resourceId and resource2Id, is it projectId and merge-requestId, or the other way around?

Solution Proposal

• Override ResourceNotes methods just to rename the arguments.

[pbmiguel]

@jdalrymple what do you think of this approach?

[jdalrymple]

There might be a way just to override the types, but im not sure.

[jdalrymple]

Something like this:

export interface MergeRequestNotes extends ResourceNotes {
  all(
    projectId: string | number,
    mergeRequestsId: string | number,
    options?: PaginatedRequestOptions,
  );
}

export class MergeRequestNotes extends ResourceNotes {
  constructor(options: BaseServiceOptions = {}) {
    super('projects', 'merge_requests', options);
  }
}

[pbmiguel]

@jdalrymple changed as you've suggested to methods: all, create, edit, remove, show

So through this approach (I think):

• It has the advantage of being more readable (as it becomes explicit what resourceId and resource2Id are about);
• However it has the disadvantage of being less maintanable (cause it creates a dependency between the interface, and the baseClass ResourceNotes. If you change ResourceNotes, you also have to change the interface too).

Nervertheless, it seems okay to me 👍

[jdalrymple]

@jdalrymple changed as you've suggested to methods: all, create, edit, remove, show

So through this approach (I think):

* It has the advantage of being more readable (as it becomes explicit what resourceId and resource2Id are about);

* However it has the disadvantage of being less maintanable (cause it creates a dependency between the interface, and the baseClass ResourceNotes. If you change ResourceNotes, you also have to change the interface too).

Nervertheless, it seems okay to me +1

Seems like a fair trade off to me. 👍

[jdalrymple]

Im going to hold off a little bit before merging it. I feel like we might as well add the interfaces for all the services that extend templates

[pbmiguel]

sure. I can do that. no problem

[pbmiguel]

@jdalrymple, added interfaces for the child classes of the following templates:

• resourceawardemojis
• projectbadges child
• resourcecustomattributes
• resourcediscussions
• resourcelabels
• resourcenotes
• projectvariables
• and resourcemilestones

[pbmiguel]

@jdalrymple, Is there anything I can do to help finalize this merge-request?

[jdalrymple]

With all the freetime this week lol I should be able to merge this in ASAP!