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

New CNCS definition. #16933

Merged
merged 2 commits into from Jul 25, 2016
Merged

New CNCS definition. #16933

merged 2 commits into from Jul 25, 2016

Conversation

AndreiSavici
Copy link
Member

New CNCS definition according to mantidgeometry branch CNCS_2016B

To test:

Please merge mantidproject/mantidgeometry#79 as well

Fixes #16932


Reviewer

Please comment on the following (full description):

Code Review
  • Is the code of an acceptable quality?
  • Does the code conform to the coding standards? Is it well structured with small focussed classes/methods/functions?
  • Are there unit/system tests in place? Are the unit tests small and test the a class in isolation?
  • If there are changes in the release notes then do they describe the changes appropriately?
Functional Tests
  • Do changes function as described? Add comments below that describe the tests performed?
  • How do the changes handle unexpected situations, e.g. bad input?
  • Has the relevant documentation been added/updated?
  • Is user-facing documentation written in a user-friendly manner?
  • Has developer documentation been updated if required?
  • Does everything look good? Comment with the ship it emoji but don't merge. A member of @mantidproject/gatekeepers will take care of it.

@AndreiSavici AndreiSavici added the Direct Inelastic Issues and pull requests related to direct inelastic label Jul 19, 2016
@AndreiSavici AndreiSavici added this to the Release 3.8 milestone Jul 19, 2016
@AndreiSavici AndreiSavici added the Patch Candidate Urgent issues that must be included in a patch following a release label Jul 19, 2016
@peterfpeterson peterfpeterson self-assigned this Jul 21, 2016
@peterfpeterson
Copy link
Member

Looks about right. A few small issues:

  • You changed filename conventions. The old one should be called CNCS_Definition_20160212-20160713.xml.
  • This was not created by Michael Reuter. Please change the name in the file.
  • Do you really mean for this to new one to change at midnight between July 13 and 14? That is a Wednesday to a Thursday (not a maintenance day) and I would have expected that they want a constant geometry for the run-cycle (starting July 1).

@peterfpeterson
Copy link
Member

All the issues were addressed. :shipit:

@FedeMPouzols FedeMPouzols merged commit d4fe7ae into master Jul 25, 2016
@FedeMPouzols FedeMPouzols deleted the 16932_CNCS_2016B branch July 25, 2016 07:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Direct Inelastic Issues and pull requests related to direct inelastic Patch Candidate Urgent issues that must be included in a patch following a release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants