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

Handle any trailing white space #23611

Merged
merged 1 commit into from Jun 20, 2018
Merged

Conversation

Dr15Jones
Copy link
Contributor

In boost 1.67, the part of the parser which finds enum values
is being passed any trailing white spaces that follow the enum.
We now explicitly remove any extra characters.

In boost 1.67, the part of the parser which finds enum values
is being passed any trailing white spaces that follow the enum.
We now explicitly remove any extra characters.
@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

@Dr15Jones
Copy link
Contributor Author

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 18, 2018

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/28740/console Started: 2018/06/18 17:31

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @Dr15Jones (Chris Jones) for master.

It involves the following packages:

CommonTools/Utils

@perrotta, @monttj, @cmsbuild, @slava77, @gpetruc, @arizzi can you please review it and eventually sign? Thanks.
@makortel this is something you requested to watch as well.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-23611/28740/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 31
  • DQMHistoTests: Total histograms compared: 2902013
  • DQMHistoTests: Total failures: 2
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2901821
  • DQMHistoTests: Total skipped: 190
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 30 files compared)
  • Checked 128 log files, 14 edm output root files, 31 DQM output files

// end
if(*(end-1) != *begin) {
--end;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

do I understand correctly that this can not be ported to another release using another boost version .. and may resurface as a problem if a future version of boost changes behavior?
Perhaps a more explicit check on the boost version is more appropriate.

Copy link
Contributor

Choose a reason for hiding this comment

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

also, I'm missing where exactly is the updated logic matching the extra white space in an explicit way.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change will work properly with boost new and old. The updated unit tests to cover the newly discovered problem checks that.

This change makes sure that the end dilimiter matches the beginning delimiter found by the parser. For the old version of boost, or when there is no trailing white space, the if is satisfied immediately.

Copy link
Contributor

Choose a reason for hiding this comment

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

ah. I should assume here already that *begin is the opening delimiter.
OK.

@slava77
Copy link
Contributor

slava77 commented Jun 19, 2018

+1

for #23611 95894c5

  • technical; additional unit tests are also provided to check for possible problems explicitly
  • jenkins tests pass and comparisons show no differences

@fabiocos
Copy link
Contributor

+1

@fabiocos
Copy link
Contributor

merge

@cmsbuild cmsbuild merged commit 8b5484a into cms-sw:master Jun 20, 2018
@Dr15Jones Dr15Jones deleted the newBoostCutParser branch June 21, 2018 17:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants