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

#164 add_display_name #164 #170

Closed
wants to merge 0 commits into from
Closed

#164 add_display_name #164 #170

wants to merge 0 commits into from

Conversation

ank1traj
Copy link

@ank1traj ank1traj commented Dec 6, 2023

PR Title: Add display_name field to student and mentor models

PR Description:

This PR adds a display_name field to student and mentor models. This field will be displayed on the website and in statistics while the name field will be printed on the certificate.

Changes:

  • Add a display_name field to the Student and Mentor models.

Benefits:

  • This change will make it easier for users to identify students and mentors on the website and in statistics.
  • It will also make it easier to distinguish between the display_name and name fields when printing certificates.

Copy link
Member

Choose a reason for hiding this comment

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

Why was this file changed?

Copy link
Author

Choose a reason for hiding this comment

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

Din't make any changes in this file. It got deleted so just added with same code.

Copy link
Member

Choose a reason for hiding this comment

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

File permission seems to have changed. Can you revert this change?

README.md Outdated Show resolved Hide resolved
@harshkhandeparkar
Copy link
Member

Have you tested this locally?

@harshkhandeparkar
Copy link
Member

harshkhandeparkar commented Dec 13, 2023

@ank1traj are you still working on this? If yes, please make the requested changes and reply to my questions. Thanks!

@harshkhandeparkar
Copy link
Member

@ank1traj ?

@ank1traj
Copy link
Author

Have you tested this locally?

Couldn't able to check due to the laptop constraints. And, sorry for missing out the above message. Didn't get the notifications before.

@harshkhandeparkar
Copy link
Member

@ank1traj The tests were recently fixed in #179. Can you please rebase on the main branch? Thanks.

@harshkhandeparkar
Copy link
Member

@ank1traj ?

@ank1traj ank1traj closed this Dec 25, 2023
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