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

manifest file name based on project breaks validate-xml #99

Closed
jthake opened this issue Nov 10, 2015 · 12 comments
Closed

manifest file name based on project breaks validate-xml #99

jthake opened this issue Nov 10, 2015 · 12 comments

Comments

@jthake
Copy link
Contributor

jthake commented Nov 10, 2015

Now that the manifest name is dynamic, validate-xml breaks because its looking for manifest.xml. @waldekmastykarz @andrewconnell

@waldekmastykarz
Copy link
Contributor

Shall I fix it @andrewconnell or do you want to pick it up?

@andrewconnell
Copy link
Contributor

It's not "broken"... it doesn't assume the XML file you want to test. It requires you to pass in an option that specifies the file you want to test (in case you have multiple in the same folder.

Ref: https://github.com/OfficeDev/generator-office/blob/master/generators/content/templates/common/gulpfile.js#L40

@jthake
Copy link
Contributor Author

jthake commented Nov 11, 2015

Is there a way to run a help command that gives more information on the gulp task to make that more obvious? and also add this to the readme page too then?

@andrewconnell
Copy link
Contributor

As is now? No... generally people just comment gulp tasks using JSDoc (which we've done... granted the validate-xml task could use some additional comments explaining usage).

An enhancement could be done to use the gulp-help plugin, but requires rework of our gulp tasks (should be done to the generator & generated projects. You can see an example of the output I generate for my projects in the screenshot here: http://www.andrewconnell.com/blog/dynamically-loading-gulp-tasks-for-simplified-reuse-and-maintenance

@waldekmastykarz
Copy link
Contributor

We could also fix the default behavior to look for manifest-*.xml files and validate them as discussed in #58 (comment)

@andrewconnell
Copy link
Contributor

Right that's what I changed it to... To require dev to pass it in. That's not a big deal IMHO...

@waldekmastykarz
Copy link
Contributor

Check. I thought the task would automatically look for any manifest-*.xml files if no specific filename has been passed. It's okay if it doesn't but then it would be better if the task failed with a clear error showing an example of the filename should be passed to the task.

Using gulp-help would be definitely a good way of documenting the different tasks particularly the ones that accept parameters such as validate-xml.

@andrewconnell
Copy link
Contributor

Let's not have multiple discussions in this thread... Two issues discussed here.

Can add an enhancement for the task to try to find a manifest in the current folder... Personally I don't like this because what if the manifest is in a sub folder? Can run gulp from the sub folder... So I'd require you pass a path to it.

As for gulp-help issue, spin up another issue as an enhancement to the generator.

@andrewconnell
Copy link
Contributor

Where are we on this... the validate-xml for the manifest. Is it fine as is? Can we close this?

@waldekmastykarz
Copy link
Contributor

My suggestion: check for manifest-*.xml in the current folder. If it isn't there, require dev to pass the path to the task explicitly.

@jthake?

@andrewconnell
Copy link
Contributor

I’d amend to say _if there isn’t exactly one manifest-_.xml then require the caller to specify the filename / path*

@waldekmastykarz
Copy link
Contributor

Yes, good idea @andrewconnell

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants