Skip to content

update packages and remove gulp-util#17

Open
prma85 wants to merge 7 commits intogulp-community:masterfrom
prma85:master
Open

update packages and remove gulp-util#17
prma85 wants to merge 7 commits intogulp-community:masterfrom
prma85:master

Conversation

@prma85
Copy link

@prma85 prma85 commented Jan 20, 2020

No description provided.

@prma85 prma85 closed this Jan 20, 2020
@phated
Copy link
Member

phated commented Jan 21, 2020

I'm open to this. Can I help resolve the issues? I'd also love to include you on the project and transfer it to "gulp community". Thoughts?

@phated phated reopened this Jan 21, 2020
@prma85
Copy link
Author

prma85 commented Jan 21, 2020

Hey @phated. Thanks for the reply.
I think it is a good idea.

The issue is basically in the template.js when running compile, it moves the /r of position and the test fails. It is probably because of the updates and need to update the tests and try it in a project to make sure.

@prma85
Copy link
Author

prma85 commented Jan 21, 2020

image

Just checking this out, and works normally in windows the test. But checking on travis, the error is just the \r not being added:

FOUND:  re,exports,module){\n\nreturn function()
WANTED: re,exports,module){\r\n\r\nreturn function()

@prma85
Copy link
Author

prma85 commented Sep 15, 2020

Hey @phated Did you have the change to take a look at this?

@phated
Copy link
Member

phated commented Sep 22, 2020

@prma85 sorry for the delay. I'm just realizing that you manually edited the template.js file instead of editing https://github.com/phated/gulp-wrap-amd/blob/master/templates/amd.jst then running npm run compile to generate the template.js file.

Could you make those changes?

@prma85
Copy link
Author

prma85 commented Sep 22, 2020

@prma85 sorry for the delay. I'm just realizing that you manually edited the template.js file instead of editing https://github.com/phated/gulp-wrap-amd/blob/master/templates/amd.jst then running npm run compile to generate the template.js file.

Could you make those changes?

I was sure that I didn't these updates. But I ran the compile again. Looks all good.

So, the issue is that when running on windows, the compile adds the /r in the template, and when running it on mac/linux (was I was using today and what is used by the circle ci), it removes the /r

I was also trying to update tap but it causes 5 tests to fail. The names for the generated template and for the expected doesn't match 🤔

@prma85
Copy link
Author

prma85 commented Sep 22, 2020

should be all good now ;)

@phated
Copy link
Member

phated commented Sep 24, 2020

@prma85 Sorry if there was a misunderstanding but you need to check in the .jst file. I don't see the updates to that file in your PR.

@prma85
Copy link
Author

prma85 commented Sep 24, 2020

@prma85 Sorry if there was a misunderstanding but you need to check in the .jst file. I don't see the updates to that file in your PR.

@phated I got what you mean
I didn't change this file
the changes were the result between generating it in the windows and in a max/linux and also update of loadash

running into in my mac removed part of the changes the changes

@phated
Copy link
Member

phated commented Sep 24, 2020

Why are we regenerating the file at all then?

@prma85
Copy link
Author

prma85 commented Sep 24, 2020

Why are we regenerating the file at all then?

This is generated when you run the compile command 🤔

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