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

Clarify how beam_center_x and beam_center_y are used #682

Merged
merged 1 commit into from
Jun 4, 2020
Merged

Conversation

phyy-nx
Copy link
Contributor

@phyy-nx phyy-nx commented Sep 9, 2019

beam_center_x and beam_center_y are only used if the depends_on field is not present. Clarify this and provide a definition of the default geometry given no dependency chain.

@prjemian
Copy link
Contributor

@zjttoefs - Do you have time review this before Friday?

@prjemian prjemian added this to the NXDL 2020.1 milestone Jan 27, 2020
@prjemian
Copy link
Contributor

@zjttoefs -- review today?

@prjemian
Copy link
Contributor

I'll merge this evening unless there is an objection.

@yayahjb
Copy link
Contributor

yayahjb commented Jan 30, 2020 via email

@prjemian
Copy link
Contributor

@yayahjb - Are you implying this needs more discussion and should not be merged at this time?

@yayahjb
Copy link
Contributor

yayahjb commented Jan 30, 2020 via email

@prjemian
Copy link
Contributor

OK, I will move this to a later milestone.

@prjemian prjemian modified the milestones: NXDL 2020.1, NXDL 2020.10 Jan 30, 2020
@zjttoefs
Copy link
Contributor

zjttoefs commented Jun 4, 2020

I agree, this is probably not the last word on the matter. But the PR helps clarify what we know and agree on.

@phyy-nx
Copy link
Contributor Author

phyy-nx commented Jun 4, 2020

Thanks. Merging.

@phyy-nx phyy-nx merged commit 9402835 into master Jun 4, 2020
@phyy-nx phyy-nx deleted the beam_center branch June 4, 2020 16:28
@prjemian
Copy link
Contributor

prjemian commented Jun 4, 2020

Thanks, all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants