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.

Sign up for GitHub

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

Jump to bottom

formatted guest/ento.views #1194

Merged
merged 1 commit into from
Jan 30, 2023
Merged

formatted guest/ento.views #1194

merged 1 commit into from
Jan 30, 2023

Conversation

ianengelbrecht
Copy link
Contributor

Hey guys, please see my email about this (Max/Grant). I've just formatted one small views file to start with. You'll see that views and viewdefs are now sorted alphabetically in the file, making them easier to find in big files like common.views. Also, for viewdefs, the name attribute is now on the same line as the opening tag, so if you fold all (Ctrl+k+3 in VS Code), you can quickly see what each one is.

Let me know if you like it, and if it doesn't break anything.

@ianengelbrecht
Copy link
Contributor Author

Here's the repo with the formatting script if you want to take a look: https://github.com/NSCF/specify-sort-views. Very raw string manipulation happening there.

@maxpatiiuk maxpatiiuk requested a review from a team January 30, 2023 14:30
maxpatiiuk
maxpatiiuk approved these changes Jan 30, 2023
View reviewed changes
Copy link
Member

@maxpatiiuk maxpatiiuk left a comment

Choose a reason for hiding this comment

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

The reformatting looks good! Thanks for doing this.
I will have one of our testers try it out in Specify just in case and then this can be merged.

@ianengelbrecht
Copy link
Contributor Author

ianengelbrecht commented Jan 30, 2023 via email

@maxpatiiuk
Copy link
Member

Just a FYI, our testers are busy doing pre-release testing for 7.8.5 today, but will get back to this pull request in the next few days. Sorry for the delay

@ianengelbrecht
Copy link
Contributor Author

ianengelbrecht commented Jan 30, 2023 via email

grantfitzsimmons
grantfitzsimmons approved these changes Jan 30, 2023
View reviewed changes
Copy link
Member

@grantfitzsimmons grantfitzsimmons left a comment

Choose a reason for hiding this comment

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

Looks great and behaves as expected. Great work @ianengelbrecht !

@maxpatiiuk maxpatiiuk merged commit bd3da81 into specify:master Jan 30, 2023
@maxpatiiuk
Copy link
Member

@ianengelbrecht looking forward to your pull request that reformats all the *.views.xml files in the config directory

@ianengelbrecht
Copy link
Contributor Author

ianengelbrecht commented Jan 30, 2023 via email

@maxpatiiuk
Copy link
Member

It might be easier for us to test when all the changes are included at once, rather than test everything again after each discipline is changed.

Given that you are using an automated tool rather than manually editing the files, if there is a bug with the reformatting, it would probably manifest itself in all the files (excluding any edge cases, and hopefully there arent any)

One thing I noticed though is that your script changes the order of tables in the view definition file. That might be something people are sensitive about (i.e, people like Collection Object being the first table followed by other important tables). Though that would become less important once we have a visual editor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@grantfitzsimmons grantfitzsimmons grantfitzsimmons approved these changes

@maxpatiiuk maxpatiiuk maxpatiiuk approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@ianengelbrecht @maxpatiiuk @grantfitzsimmons