PR-531: Renaming of files, functions, and macros


#1

Pull request 531 was recently filed in the PBS Pro repository… https://github.com/PBSPro/pbspro/pull/531

The PR changes the names of several components associated with linked lists, commonly used throughout PBS Pro. The PR does not contain any functional changes to the code that I am able to ascertain.

My question to the development community is whether these changes are seen as generally useful. Speaking as someone who has worked with the software for years and is familiar with the current names, the prospect of changing them is not entirely appealing. However, if the community feels there is value to the changes I’m willing to accept them.

Please state your preference in this thread as to whether the PR should be accepted. Thanks to @minghui for proposing the change.


#2

@mkaro – thanks for posting this! I would be great to get some additional input from others working on the code.

From my perspective, these changes certainly increase readability of the code – a very good thing.


#3

I agree in principle about accepting any refactoring that helps readability, however, this does not seem making this more readable to me.

I went through the change and I do not like the renaming of pbs_list_head to pbs_list_node. The current name, i.e., pbs_list_head actually is more readable, and tells me that we are passing a pointer to the head of the linked list, and not some random node in the linked list. Of course, the variable itself is called phead (in most places) signifying that it is indeed the head of the linked list, but having the type itself point that out is actually more readable (ie, if you are looking at prototypes which do not include the parameter name).

In fact the current code defines two types to make things more readable…
typedef struct pbs_list_link {
struct pbs_list_link *ll_prior;
struct pbs_list_link *ll_next;
void *ll_struct;
} pbs_list_link;
typedef pbs_list_link pbs_list_head;

Anyways, this part is really code review comments, so I will post this in the github code review itself.


#4

I tend to agree with @subhasisb. We’ll allow some time for additional comments. Any thoughts @minghui?


#5

Any more comments on changing the linked list structure names? I’m really on the fence with this change. Without additional support from developers, I’m inclined to suggest it be closed. If anyone has an opinion or technical argument (for or against) please offer it here.