Skip to content

Conversation

@pablo-garay
Copy link
Collaborator

No description provided.

@pablo-garay pablo-garay requested a review from chtruong814 June 18, 2025 21:21
Copy link
Collaborator

@chtruong814 chtruong814 left a comment

Choose a reason for hiding this comment

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

Thanks for this. Had some comments around the naming. Feel free to manually take care of that here but ideally we also clean up the template too.

__version__ = ".".join(map(str, VERSION[:3])) + "".join(VERSION[3:])

__package_name__ = "vfm"
__contact_names__ = "VFM"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry, I should have made the template better. Could you update the template so that __contact_names__ = "NVIDIA"?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done


__package_name__ = "vfm"
__contact_names__ = "VFM"
__contact_emails__ = "[email protected]"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's update template so this is __contact_emails__ = "[email protected]" for now. I can double check later if we should be using something different.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

vfm/__init__.py Outdated
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
from vfm.package_info import (
Copy link
Collaborator

Choose a reason for hiding this comment

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

When using the cookie cutter template, instead of just vfm, let's have project name be NeMo VFM. The repo name will just be VFM. But the package and project would be referenced as NeMo VFM or nemo_vfm, etc...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

Copy link
Collaborator

@chtruong814 chtruong814 left a comment

Choose a reason for hiding this comment

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

Sorry, we're not terribly consistent on the copyright in the template. Would you please resolve those and we can go ahead and get this merged?

@@ -0,0 +1,26 @@
# Copyright (c) 2025, NeMo VFM.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry, the template must be wrong here. Could you review the copyright in the template to make sure it's set to

Copyright (c) {{cookiecutter.year}}, NVIDIA CORPORATION.  All rights reserved.

@@ -0,0 +1,35 @@
# Copyright (c) 2025, NeMo VFM.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Another place to be updating the copyright template

Copyright (c) {{cookiecutter.year}}, NVIDIA CORPORATION.  All rights reserved.

@@ -0,0 +1,18 @@
# Copyright (c) 2025, NeMo VFM.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Another place to be updating the copyright template

Copyright (c) {{cookiecutter.year}}, NVIDIA CORPORATION.  All rights reserved.

@@ -0,0 +1,14 @@
# Copyright (c) 2025, NVIDIA CORPORATION.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Another place to be updating the copyright template

Copyright (c) {{cookiecutter.year}}, NVIDIA CORPORATION.  All rights reserved.

@@ -0,0 +1,14 @@
# Copyright (c) 2025, NVIDIA CORPORATION.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Another place to be updating the copyright template

Copyright (c) {{cookiecutter.year}}, NVIDIA CORPORATION.  All rights reserved.

@@ -0,0 +1,38 @@
# Copyright (c) 2025, NVIDIA CORPORATION.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Another place to be updating the copyright template

Copyright (c) {{cookiecutter.year}}, NVIDIA CORPORATION.  All rights reserved.

@@ -0,0 +1,252 @@
# Copyright (c) 2025, NVIDIA CORPORATION.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Another place to be updating the copyright template

Copyright (c) {{cookiecutter.year}}, NVIDIA CORPORATION.  All rights reserved.

@@ -0,0 +1,40 @@
# Copyright (c) 2025, NVIDIA CORPORATION.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Another place to be updating the copyright template

Copyright (c) {{cookiecutter.year}}, NVIDIA CORPORATION.  All rights reserved.

@@ -0,0 +1,83 @@
# Copyright (c) 2025, NVIDIA CORPORATION.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Another place to be updating the copyright template

Copyright (c) {{cookiecutter.year}}, NVIDIA CORPORATION.  All rights reserved.

@@ -0,0 +1,251 @@
# Copyright (c) 2025, NVIDIA CORPORATION.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Another place to be updating the copyright template

Copyright (c) {{cookiecutter.year}}, NVIDIA CORPORATION.  All rights reserved.

@pablo-garay pablo-garay merged commit aefceda into main Jun 24, 2025
2 checks passed
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.

3 participants