Skip to content

Added Department in category for Dataverse #2829#3983

Merged
kcondon merged 2 commits intodevelopfrom
2829-Add-department
Jul 10, 2017
Merged

Added Department in category for Dataverse #2829#3983
kcondon merged 2 commits intodevelopfrom
2829-Add-department

Conversation

@rbhatta99
Copy link
Copy Markdown
Contributor

@rbhatta99 rbhatta99 commented Jul 6, 2017

Related Issues

Pull Request Checklist

@coveralls
Copy link
Copy Markdown

coveralls commented Jul 6, 2017

Coverage Status

Coverage increased (+0.002%) to 10.336% when pulling 6bdc71e on 2829-Add-department into 482c2dc on develop.

@rbhatta99 rbhatta99 mentioned this pull request Jul 6, 2017
Closed
mheppler
mheppler reviewed Jul 6, 2017
View reviewed changes
Comment thread src/main/java/edu/harvard/iq/dataverse/Dataverse.java Outdated

/**
* @todo Don't hard code these as English.
* @todo Don't hard code these as English. Use bundleUtils
Copy link
Copy Markdown
Contributor

@mheppler mheppler Jul 6, 2017

Choose a reason for hiding this comment

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

@rbhatta99 It looks like you can remove this todo if you add bundle references to the "Laboratory" and "Research Group" text in this same function you reference the bundle for "Department" in line 128.

Copy link
Copy Markdown
Contributor Author

@rbhatta99 rbhatta99 Jul 6, 2017

Choose a reason for hiding this comment

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

I can do that. I can actually change all of them if need be, as they all are given in the bundle. Should I do it in this branch only?

Copy link
Copy Markdown
Contributor

@mheppler mheppler Jul 6, 2017

Choose a reason for hiding this comment

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

Yeah, all of them would be great. Sorry, GitHub only displayed a snippet of truncated code, so I only referenced those two.

Copy link
Copy Markdown
Contributor Author

@rbhatta99 rbhatta99 Jul 6, 2017

Choose a reason for hiding this comment

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

Sure, il do that!

@mheppler
Copy link
Copy Markdown
Contributor

mheppler commented Jul 6, 2017

@rbhatta99 I added a comment to your pull request in regards to a todo in Dataverse.java. @scolapasta might also be able to confirm this improvement as well.

@rbhatta99
Copy link
Copy Markdown
Contributor Author

rbhatta99 commented Jul 6, 2017

@mheppler I completed the todo in dataverse.java
@jggautier reorganised the categories list alphabetically

mheppler
mheppler approved these changes Jul 6, 2017
View reviewed changes
Copy link
Copy Markdown
Contributor

@mheppler mheppler left a comment

Choose a reason for hiding this comment

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

Looks good!

@coveralls
Copy link
Copy Markdown

coveralls commented Jul 6, 2017

Coverage Status

Coverage decreased (-0.0002%) to 10.334% when pulling 74552c3 on 2829-Add-department into 482c2dc on develop.

@kcondon kcondon merged commit 9e39c6e into develop Jul 10, 2017
@kcondon kcondon deleted the 2829-Add-department branch July 10, 2017 19:35
@pdurbin pdurbin added this to the 4.7.1 - Dashboard milestone Sep 26, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@mheppler mheppler mheppler approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

4.7.1 - Dashboard

Development

Successfully merging this pull request may close these issues.

5 participants

@rbhatta99 @coveralls @mheppler @pdurbin @kcondon
⚡