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

add a public accessor for the definition of "invalid module id" #1750

Merged
merged 2 commits into from Dec 12, 2013

Conversation

fwyzard
Copy link
Contributor

@fwyzard fwyzard commented Dec 10, 2013

No description provided.

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @fwyzard (Andrea Bocci) for CMSSW_7_0_X.

add a public accessor for the definition of "invalid module id"

It involves the following packages:

DataFormats/Provenance

@cmsbuild, @Dr15Jones, @ktf, @nclopezo can you please review it and eventually sign? Thanks.
@wmtan this is something you requested to watch as well.
You can sign-off by replying to this message having '+1' in the first line of your reply.
You can reject by replying to this message having '-1' in the first line of your reply.
@ktf you are the release manager for this.

@fwyzard
Copy link
Contributor Author

fwyzard commented Dec 10, 2013

rebased on top of CMSSW_7_0_X_2013-12-10-0200

@cmsbuild
Copy link
Contributor

Pull request #1750 was updated. @cmsbuild, @Dr15Jones, @ktf, @nclopezo can you please check and sign again.

{
return std::numeric_limits<unsigned int>::max();
}

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd prefer the function to be called invalidID(). I known the other static has get in its name, but that was becuase it changes each time it is called (a better name would have been getNewUniqueID().

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, best to move the body of the function within the #if ... #else section and then just use ; for the second declaration. That way Reflex also never has to see std::numeric_limits.

@cmsbuild
Copy link
Contributor

Pull request #1750 was updated. @cmsbuild, @Dr15Jones, @ktf, @nclopezo can you please check and sign again.

return std::numeric_limits<unsigned int>::max();
}
#else
static unsigned int invalidID() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Just make it undefined (i.e. don't give it any body). This is only seen by parser and never actually gets compiled.

@cmsbuild
Copy link
Contributor

@fwyzard
Copy link
Contributor Author

fwyzard commented Dec 11, 2013

Done.

.A

@cmsbuild
Copy link
Contributor

Pull request #1750 was updated. @cmsbuild, @Dr15Jones, @ktf, @nclopezo can you please check and sign again.

@Dr15Jones
Copy link
Contributor

+1
Thanks.

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next IBs unless changes or unless it breaks tests. @ktf can you please take care of it?

ktf added a commit that referenced this pull request Dec 12, 2013
Framework -- Add a public accessor for the definition of "invalid module id"
@ktf ktf merged commit cecb06f into cms-sw:CMSSW_7_0_X Dec 12, 2013
@cmsbuild
Copy link
Contributor

@fwyzard fwyzard deleted the add_invalid_module_id branch December 12, 2013 14:07
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