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

Refactor the XSD so that the attribute ordering is the same as listed in the style guide #74

Closed
demcg opened this issue Apr 2, 2015 · 15 comments
Assignees
Milestone

Comments

@demcg
Copy link
Contributor

demcg commented Apr 2, 2015

The attributes in the XSD files should be ordered in the following manner:

  1. name
  2. type
  3. minOccurs
  4. maxOccurs

This is a change that will create many small changes all over the file, so it should be merged when there are not too many open branches with major refactors on them.

@demcg demcg added this to the Version 5.0 milestone Apr 2, 2015
@demcg demcg self-assigned this Apr 2, 2015
@jungshadow
Copy link
Collaborator

This is a great idea and it may be worth capturing now if you're willing to do it, @demcg. I would try to hammer out #75 and #76 at the same time, once we decide those.

@pstenbjorn @jdmgoogle @cjerdonek Sound good to you? If so, let's freeze code changes for a day or two while @demcg is working on this.

@pstenbjorn
Copy link
Contributor

@jungshadow yes, this is needed standardization of the xsd.

@cjerdonek
Copy link
Contributor

@jungshadow Yes, freezing sounds fine. My only suggestion is to do each "type" of change as a separate PR to make them easier to review.

By the way, there may be more after this if we think of any (e.g. the ordering of type definitions, both where to put them in the doc and ordering them within).

@cjerdonek
Copy link
Contributor

By the way, one thing that should be clarified in the guide is that anything that isn't in the list of four things should go afterwards. In particular, this would mean that "use" would go after "name" and "type". (I'm assuming that's how we would want to do it.)

@demcg
Copy link
Contributor Author

demcg commented Apr 3, 2015

I can start this when we hash out #75 and #76

@demcg
Copy link
Contributor Author

demcg commented Apr 3, 2015

@cjerdonek I agree that "use" should be after "type" and I can create an issue for this and any other change to the style guide

@cjerdonek
Copy link
Contributor

@demcg I would prefer each issue be done separately so it is easier to review. Also, that way you can start this one without having to wait.

@demcg
Copy link
Contributor Author

demcg commented Apr 3, 2015

@cjerdonek sounds good, I will start with #74 and get a PR up for it soon

@demcg
Copy link
Contributor Author

demcg commented Apr 3, 2015

@cjerdonek, @jungshadow
I noticed a small nit to pick about spacing ... should we have a space after the last attribute and before that last angle bracket?

for example:
with space - ... maxOccurs="1" />
without space - ... maxOccurs="1"/>

We have both patterns, but more than 50% with space

@cjerdonek
Copy link
Contributor

@demcg I would say no space. Something else worth including in the guide.

@jungshadow
Copy link
Collaborator

@cjerdonek @demcg Again, not to start a flame war, but, since I was raised in the old school XHTML, I'd rather have that extra visual space after the final attribute.

@cjerdonek
Copy link
Contributor

@jungshadow That's fine with me. I don't care too much. 😀

@demcg
Copy link
Contributor Author

demcg commented Apr 3, 2015

Sounds good I will make sure that 1 space is between the last attribute and the closing of the tag

cjerdonek added a commit to cjerdonek/vip-specification that referenced this issue Apr 3, 2015
These were discussed in the comments to issue votinginfoproject#74.
cjerdonek added a commit to cjerdonek/vip-specification that referenced this issue Apr 3, 2015
These were discussed in the comments to issue votinginfoproject#74.
@cjerdonek
Copy link
Contributor

I created PR #79 to update the style guide per our comments. The rendered form can be viewed here.

cjerdonek added a commit that referenced this issue Apr 3, 2015
…guide

Issue #74: Add more style guidelines re: attributes
@demcg
Copy link
Contributor Author

demcg commented Apr 3, 2015

This issues is linked to PR #80

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

No branches or pull requests

4 participants