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.

Sign up for GitHub

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

Jump to bottom

[M] 1476028: Changed serial number datatype to String (ENT-528) #2523

Merged
merged 1 commit into from Dec 23, 2019
Merged

[M] 1476028: Changed serial number datatype to String (ENT-528) #2523

merged 1 commit into from Dec 23, 2019

Conversation

abhiskum
Copy link
Contributor

  • Updated serialnumber field datatype from Long to String in CertificateSerialDTO
  • Updated serial field datatype form Long to String in CertificateSerialDto
  • Converted Long serial value to String from CertificateSerial and set it to CertificateSerialDTO
  • Converted Long id to String and pass it to CertificateSerialDto
  • Updated Junit and rspec with respect to this change

@abhiskum
Copy link
Contributor Author

We need ack from Candlepin API consumers before merging this into master.

@Ceiu
Copy link
Contributor

Ceiu commented Dec 10, 2019

@candlepin-bot retest this please

@abhiskum abhiskum force-pushed the abhiskum/change_serial_number_to_string branch 3 times, most recently from b1ebf34 to fff2aea Compare December 13, 2019 10:12
Ceiu
Ceiu requested changes Dec 13, 2019
View reviewed changes
Copy link
Contributor

@Ceiu Ceiu left a comment

Choose a reason for hiding this comment

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

Almost ready; just one last standardization thing and a question regarding one of the casts/types.

server/spec/cert_revocation_spec.rb Outdated Show resolved Hide resolved
server/spec/client_v1_size_spec.rb Show resolved Hide resolved
@abhiskum abhiskum force-pushed the abhiskum/change_serial_number_to_string branch from fff2aea to 63a2ffa Compare December 16, 2019 07:07
@abhiskum
Copy link
Contributor Author

@candlepin-bot Please retest

@abhiskum
Copy link
Contributor Author

@candlepin-bot retest this please

@Ceiu
Copy link
Contributor

Ceiu commented Dec 16, 2019

@candlepin-bot retest this please

@abhiskum
Copy link
Contributor Author

@candlepin-bot retest this please

@nikosmoum
Copy link
Member

nikosmoum commented Dec 19, 2019 •
edited

@abhiskum Code is good, but we probably want to squash the 2 commits into 1, and have the bugzilla id in the commit summary line / PR title, to preserve historical documentation, before merging (and so that the release build can auto-close the bz based on the commit message).

@abhiskum abhiskum force-pushed the abhiskum/change_serial_number_to_string branch from c41da64 to 9c0a512 Compare December 23, 2019 06:56
@abhiskum abhiskum changed the title ENT-528: Changed serial number datatype to String [M] 1476028: Changed serial number datatype to String (ENT-528) Dec 23, 2019
@abhiskum
Copy link
Contributor Author

@nikosmoum squashed commit and added bug number.

@abhiskum
1476028: ENT-528: Changed serialnumber datatype to String
6be9ea3 
  - Updated serialnumber field datatype from Long to String in CertificateSerialDTO
  - Updated serial field datatype form Long to String in CertificateSerialDto
  - Converted Long serial value to String from CertificateSerial and set it to CertificateSerialDTO
  - Converted Long id to String and pass it to CertificateSerialDto
  - Updated Junit and rspec with respect to this change
@abhiskum abhiskum force-pushed the abhiskum/change_serial_number_to_string branch from 9c0a512 to 6be9ea3 Compare December 23, 2019 07:35
@nikosmoum nikosmoum merged commit 1256b2f into candlepin:master Dec 23, 2019
@sonalidhome sonalidhome mentioned this pull request Feb 12, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nikosmoum nikosmoum nikosmoum approved these changes

@Ceiu Ceiu Ceiu approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@abhiskum @Ceiu @nikosmoum