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

Some updates #40

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Some updates #40

wants to merge 7 commits into from

Conversation

e965
Copy link

@e965 e965 commented Mar 13, 2019

  • Changed the link for js-yaml lib (because RawGit will close soon)
  • "build" task is fixed (some file paths were wrong)
  • XMLHttpRequest is replaced by fetch

Dmitry and others added 2 commits March 13, 2019 13:20
Because [RawGit](https://rawgit.com) will close soon.
"build" task is fixed, XMLHttpRequest is replaced by fetch
@e965 e965 changed the title Changed the link for js-yaml lib Some updates Mar 13, 2019
¯\_(ツ)_/¯
@Pizzacus
Copy link
Owner

Okay, so I wanna clarify that both fetch wasn't used and yaml is imported like that for the same reason, back when I made this, satania.moe wasn't running on webpack (actually, I don't think webpack was even popular back then) so I did not use fetch to ensure we were compatible with IE (which is still used a lot by our Chinese visitors) and imported YAML from an URL because I had no other ways to manage dependencies back then lol.

Now it would be better to put YAML as a dependency and add a fetch pollyfill.

Also I'm not sure about what you're doing with gulp-rename there?

Fix previous fix (with build paths issue), added fetch and js-yaml polyfills
@e965
Copy link
Author

e965 commented Mar 13, 2019

Also I'm not sure about what you're doing with gulp-rename there?

In the current version of the project, the build paths of some files are broken (e.g. ./dist/src/assets/favicon.jpg instead of ./dist/favicon.jpg). I fixed it.

@Pizzacus
Copy link
Owner

(forget the last message I didn't get what you meant)

No this is the intended file structure.

As with most Gulp projects, things in src/ go into dist/, I put assets at the root just because there is no reason not to do that in the built website

@Pizzacus
Copy link
Owner

Pizzacus commented Mar 13, 2019

Or if you meant that for you, things go into ./dist/src/, well, that's weird because it doesn't do it for me

image

(I'm not sure if you meant by "(e.g. ./dist/src/assets/favicon.jpg instead of ./dist/favicon.jpg)" that it's what the change does or if it's what the current script does)

@e965
Copy link
Author

e965 commented Mar 13, 2019

¯\(ツ)

(node 10.12.0, npm 6.4.1)

@Pizzacus
Copy link
Owner

Pizzacus commented Mar 13, 2019

Is your working dir (the dir you are cd into) the root of the satania.moe dir?

No wait it does look like it (I am really terrible today oh my god)

@e965
Copy link
Author

e965 commented Mar 13, 2019

My working dir is the one into which I have cloned the git repository.

@Pizzacus
Copy link
Owner

Pizzacus commented Mar 13, 2019

I have found the problem, it's the use of path.join on globs that makes Gulp confused.

Normally, Gulp can find the "parent" of a Glob, so for instance, in owo/uwu/**/*.js, the parent is owo/uwu, so it is not added when you do gulp.dest.

But the Globs are generated with path.join so on windows, that separates folders with the backslas, while Globs always separate folders with the forward slash.

Solution: path.join will no longer be used to create globs, Gulp's cwd option will be used to do it properly

This will be fixed in an upcoming commit

Pizzacus added a commit that referenced this pull request Mar 13, 2019
As mentionned on #40
outloudvi pushed a commit to suisei-cn/hosimati.suisei.moe that referenced this pull request May 1, 2020
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.

2 participants