Skip to content

update packages and remove gulp-util - #17

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

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

Conversation

@prma85

@prma85 prma85 commented Jan 20, 2020

Copy link
Copy Markdown

No description provided.

@prma85 prma85 closed this Jan 20, 2020
@phated

phated commented Jan 21, 2020

Copy link
Copy Markdown
Member

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

prma85 commented Jan 21, 2020

Copy link
Copy Markdown
Author

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

prma85 commented Jan 21, 2020

Copy link
Copy Markdown
Author

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

prma85 commented Sep 15, 2020

Copy link
Copy Markdown
Author

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

@phated

phated commented Sep 22, 2020

Copy link
Copy Markdown
Member

@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

prma85 commented Sep 22, 2020

Copy link
Copy Markdown
Author

@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

prma85 commented Sep 22, 2020

Copy link
Copy Markdown
Author

should be all good now ;)

@phated

phated commented Sep 24, 2020

Copy link
Copy Markdown
Member

@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

prma85 commented Sep 24, 2020

Copy link
Copy Markdown
Author

@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

phated commented Sep 24, 2020

Copy link
Copy Markdown
Member

Why are we regenerating the file at all then?

@prma85

prma85 commented Sep 24, 2020

Copy link
Copy Markdown
Author

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