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

DS-2656: Tidy up whitespaces, tabs, etc in input-forms.xml #1158

Closed
wants to merge 1 commit into from

Conversation

alanorth
Copy link
Contributor

Generated with the following command:

$ tidy -xml -iq -m -w 0  dspace/config/input-forms.xml

Results in no functional changes, only aesthetic.

Generated with the following command:

    $ tidy -xml -iq -m -w 0  dspace/config/input-forms.xml

Results in no functional changes, only aesthetic.

Signed-off-by: Alan Orth <alan.orth@gmail.com>
@helix84
Copy link
Member

helix84 commented Nov 13, 2015

I don't like the blank lines removed, that makes readability worse.

@helix84 helix84 added the code task Code cleanup task label Nov 13, 2015
@helix84 helix84 added this to the 6.0 milestone Nov 13, 2015
@alanorth
Copy link
Contributor Author

To be fair, it's not meant to be readable to you—it's machine-readable XML. ;)

In our institute I run tidy after every modification I make to our subject terms, before committing to the repo.

@helix84
Copy link
Member

helix84 commented Nov 13, 2015

The machine doesn't care about this whitespace change in the slightest. The humans who edit it probably do. There's merit in making it the file readable, which is why you are making this PR. I don't object to formatting it, just to one specific formatting change.

@alanorth
Copy link
Contributor Author

I guess it depends who is editing it. I'm a systems admin, and I edit it in vim in a terminal. It's actually because of our librarians editing it and introducing all manner of white space errors that I started formatting it in the first place!

@mwoodiupui
Copy link
Member

I must agree that I'd prefer not to lose the vertical whitespace.

@tdonohue tdonohue added the needs discussion Ticket or PR needs discussion before it can be moved forward. label Mar 30, 2016
@tdonohue tdonohue removed this from the 6.0 milestone Mar 30, 2016
Copy link
Member

@tdonohue tdonohue left a comment

Choose a reason for hiding this comment

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

Agreed with others that the veritical whitespace would be nice to keep. I'm OK with cleaning up tabbing, trailing spaces though, if this PR was updated.

@alanorth
Copy link
Contributor Author

It's ok, @tdonohue. I will close this pull request and just continue to format / audit our file for correctness with tidy locally. My main gripe was that our once-yearly rebase on top of new DSpace release branches cause tons of merge conflicts due to whitespace.

@alanorth alanorth closed this Feb 22, 2017
@alanorth alanorth deleted the DS-2656 branch February 22, 2017 12:07
4science-it pushed a commit to 4Science/DSpace that referenced this pull request Oct 17, 2023
[DSC-1278]  fixed for authority metadata import with CSV

Approved-by: Stefano Maffei
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code task Code cleanup task needs discussion Ticket or PR needs discussion before it can be moved forward.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants