-
Notifications
You must be signed in to change notification settings - Fork 41
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
append current time to copied project display name #4331
append current time to copied project display name #4331
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor recommendation for copied project display_name.
lib/project_copier.rb
Outdated
current_timestamp = Time.now.utc.strftime('%Y-%m-%d %H:%M:%S') | ||
copied_project.display_name += ' (copy)' if user == project_to_copy.owner | ||
copied_project.display_name += " #{current_timestamp}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Following method used in workflow_copier.rb (see workflow_copier.rb#L22), and removing if user == owner
(that doesn't seem necessary since all workflows will have a unique copy
tag added.
current_timestamp = Time.now.utc.strftime('%Y-%m-%d %H:%M:%S') | |
copied_project.display_name += ' (copy)' if user == project_to_copy.owner | |
copied_project.display_name += " #{current_timestamp}" | |
copied_project.display_name += " (copy: #{Time.now.utc.strftime('%Y-%m-%d %H:%M:%S')})" |
spec/lib/project_copier_spec.rb
Outdated
expect(copied_project.display_name).to eq(project.display_name) | ||
it 'appends current timestamp to display_name' do | ||
allow(Time).to receive(:now).and_return(Time.utc(2024, 5, 17, 12, 0, 0)) | ||
expect(copied_project.display_name).to eq("#{project.display_name} 2024-05-17 12:00:00") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If suggestion above is used, then here is the fix for the spec.
expect(copied_project.display_name).to eq("#{project.display_name} 2024-05-17 12:00:00") | |
expect(copied_project.display_name).to eq("#{project.display_name} (copy: 2024-05-17 12:00:00)") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
FYI: you can choose to "commit suggestion" from review comments when they are available -- that might be faster than creating those edits / commits on your own. |
Yeah, i'll do going forward. Just wanted to reconfirm with the tests locallt |
Describe your change here.
Review checklist
apiary.apib
file?