Skip to content

Conversation

@JaafarMehrez
Copy link
Member

In this pull request, two folders (AE, FC) corresponding to "All Electron" and "Frozen Core" calcualtions were created under the (NR) folder. Within each of the created folders, a new folder "CFOR-CFOUR" was created which contains the output file of the calculations, the .txt files were named based on the basis set used for the calculation. As an example "aCV5Z-EMSL_CCSDpT.txt" corresponds to all electron calculation using aCV5Z-EMSL basis set within CCSD(T) level of theory.

@ndattani
Copy link
Member

Thanks @JaafarMehrez. Why did you make three separate commits instead of just one?

@JaafarMehrez
Copy link
Member Author

@ndattani. I thought that this way would make the commits more detailed and add clarity.

.DS_Store Outdated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JaafarMehrez please remove .DS_Store. I ask everyone to use Git from a terminal, rather than from a Mac folder.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ndattani I have cleaned up the config files like .DS_Store. That file was accidentally pushed to the repository, this happened long time ago, at that time I forgot to add the .DS_Store file to the .gitignore

O+/+o_table.pdf Outdated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JaafarMehrez please just name the files table.pdf and table.typ, since they're already in a folder called O+.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ndattani I have changed the names of the files to be table.typ in each folder.

O/o_table.typ Outdated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mentioned in Discord that the basis sets that you used need to be labeled with CV rather than just V.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ndattani The lables used for the basis sets has been changed to include CV rather than just V.

@ndattani
Copy link
Member

@JaafarMehrez It looks like this pull request can be approved and merged into the main branch once the comments that I made today are addressed!

@ndattani
Copy link
Member

Hi @JaafarMehrez, on 25 February 2025 (5 months ago), I wrote that this pull request can be approved once the comments were addressed. Were they all addressed? I see that there's a couple commits that were made 4 months ago that were titled "Addressed comments on PR" but there hasn't been any direct communication indicating that each of the comments have been addressed (this may have been okay if they were done on 26 February 2025, but 5 months or 1 month later it's hard for me to assess the status of the pull request the way I would if the commits were made sooner after the comments were made).

@JaafarMehrez
Copy link
Member Author

Hi @ndattani, all the comments are addressed, I have cleaned up the config files like .DS_Store, actually all I am doing is from the terminal, but because I config my termianl to always show hidden files, that file was accidentally pushed to the repo, this happened long time ago, at that time I forgot to add the .DS_Store file to the .gitignore

For the other two comments, I have changed the names of the files to be table.typ in each folder. I have also updated the lables of the basis sets to include CV.

@ndattani
Copy link
Member

ndattani commented Aug 5, 2025

Hi @JaafarMehrez, please address each comment individually on the comment thread itself. because otherwise it's harder to keep track of things. There's also a button for me to click that looks like this:
DA2C4770-CD6C-4382-A5D3-70458163F346, which I would not click until the comment is addressed. Many repositories, for example the #OpenMolcas one on GitLab, do not even allow the pull request to be approved without all conversations being "resolved" in that type of way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants