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

Shared resources #696

Merged
merged 14 commits into from
Jun 30, 2021
Merged

Shared resources #696

merged 14 commits into from
Jun 30, 2021

Conversation

duncdrum
Copy link
Collaborator

@duncdrum duncdrum commented Jun 27, 2021 •
edited
Loading

This removes shared-resources from yeoman generated app templates. it also switches to the new templating library OOTB.

TODO: 35 tests are currently skipped, these need to be reactivated and not failing before this is in good shape. The current error is

Error: Unexpected close tag
Line: 4
Column: 18
Char: >

across a number of files, the cause is still unclear to me, none of the changes should have anything to d with making xml such as .xconf invalid, i haven't even touched collection.xconf

see eXist-db/exist#3914

close #36
close #697
See #11

@duncdrum
fix(var): fix corrupted build.xml
1ae66bf 
enable all test
treeshake
@duncdrum
Copy link
Collaborator Author

progress #697 is still holding this back.

duncdrum added 2 commits June 28, 2021 16:47
@duncdrum
chore(bootstrap): bump bs 5
7ad6a25 
see eXist-db#697
@duncdrum
fix(exist-2.2css): ignore overwritten css rules
e5f2354 
fixing them leads to too much breakage involed

adjust linter
see eXist-db#697
see #36
@duncdrum duncdrum marked this pull request as ready for review June 30, 2021 11:10
@duncdrum duncdrum requested review from joewiz and a team June 30, 2021 11:12
line-o
line-o reviewed Jun 30, 2021
View reviewed changes
Copy link
Member

@line-o line-o left a comment

Choose a reason for hiding this comment

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

The title of this PR could be "Exclusive Resources" :)
Good work, but I haven't tested it locally.
I like the addition of linting and autofixing issues. Is that set-up for the generated app, this tool or both?

Remarks:

  • Some files in the generators/app/templates/exist-design/img folder has some files in it that I think are just leftover cruft. If there is a valid reason to keep all of them then it is fine.
  • generators/app/templates/img/powered-by.svg should be used optimised. Whether to have an unoptimised version next to it is a separate decision. But the filename should be obvious (something like powered-by.inkscape.svg or .source.svg)
  • From what I read this is still creating an app with ant as the build tool. Or are there more options already but they had not to change?

@duncdrum
Copy link
Collaborator Author

duncdrum commented Jun 30, 2021 •
edited
Loading

the images are a full-download from shared-resources I don't think it is worth sorting out which ones are no longer called by the css, since that will go the way of the dodo soon see #698 .

same reason i kept the svg unaltered, once the style sheet gets a revamp, and the css linting code goes into the generated apps i ll take another look. chances are the generated apps will also include the bootstrap icon library to keep things tidy and easy to extend based on their documentation.

this just ant, the gulp changes are still WIP, and unrelated.

line-o
line-o approved these changes Jun 30, 2021
View reviewed changes
Copy link
Member

@line-o line-o left a comment

Choose a reason for hiding this comment

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

Good work.

@line-o line-o merged commit b578495 into eXist-db:master Jun 30, 2021
@github-actions
Copy link

🎉 This PR is included in version 5.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

@line-o line-o line-o approved these changes

@joewiz joewiz Awaiting requested review from joewiz

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

Successfully merging this pull request may close these issues.

ditch jquery all together an longjump to bootstrap@5 make shared resources optional
3 participants
@duncdrum @joewiz @line-o