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

Removed obsolete and unnecessary files. #2809

Merged
merged 5 commits into from Dec 9, 2016
Merged

Conversation

kc284
Copy link
Contributor

@kc284 kc284 commented Oct 24, 2016

Html for the scripts does not need to be in source control as it can be
autogenerated using various tools (e.g. pygmentize, enscript etc; the scripts
are even automatically highlighted on github).

Signed-off-by: Konstantina Chremmou konstantina.chremmou@citrix.com

@robhoes
Copy link
Member

robhoes commented Oct 26, 2016

Note: I still need to review this, but in any case, we cannot put this in the master branch at this point in time.

@lindig
Copy link
Contributor

lindig commented Oct 28, 2016

I have marked this blocked because we cannot merge to trunk-ring3 at the moment.

@jonludlam
Copy link
Contributor

Over to @robhoes to review :-)

install-debian

[3][ browse ] [4][ download ]
[install-debian][1]
Copy link
Member

Choose a reason for hiding this comment

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

Is this supposed to be markdown? I don't think the syntax is right.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's markdown recognisable by github if the file has .md extension - which I've just realised this file does not have, so thanks for pointing it out, I'll rename it.

9. file://localhost/bind/myrepos/xen-api/scripts/examples/bash-cli/move-management-to-bond.html
10. file://localhost/bind/myrepos/xen-api/scripts/examples/bash-cli/move-management-to-bond

[1]: install-debian
Copy link
Member

Choose a reason for hiding this comment

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

Is this useful?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Apologies, I don't understand your question. Are you asking whether this particular example is useful or whether it's useful to have a link to the example in the readme file?

@@ -1,110 +0,0 @@
<html>
Copy link
Member

Choose a reason for hiding this comment

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

I think that removing this file will break some targets in the ocaml/idl/OMakefile (e.g. noarch-install).

Copy link
Member

Choose a reason for hiding this comment

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

Also the other html files in ocaml/idl: do they have replacements somewhere? Or is that not needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe these files used to be needed for the content on the old community page, but they're out of date and I don't think they're currently published anywhere. In any case I'll have another look to ensure the changes are not breaking anything.

Html for the scipts does not need to be in source control as it can be
autogenerated using various tools (e.g. pygmentize, enscript etc; the scripts
are even automatically highlighted on github).

Signed-off-by: Konstantina Chremmou <konstantina.chremmou@citrix.com>
…rrectly.

Signed-off-by: Konstantina Chremmou <konstantina.chremmou@citrix.com>
Signed-off-by: Konstantina Chremmou <konstantina.chremmou@citrix.com>
The old name sdk-install was giving the impression they were called from the
homonymous rule further up the folder hierarchy, while they are actually a
prerequisite of noarch-install.

Signed-off-by: Konstantina Chremmou <konstantina.chremmou@citrix.com>
Signed-off-by: Konstantina Chremmou <konstantina.chremmou@citrix.com>
@robhoes
Copy link
Member

robhoes commented Dec 9, 2016

The old name sdk-install was giving the impression they were called from the
homonymous rule further up the folder hierarchy, while they are actually a
prerequisite of noarch-install.

But I think it was called from both. I.e. from make sdk-install as well. No?

@robhoes
Copy link
Member

robhoes commented Dec 9, 2016

It looks to me, however, that the sdk-install make target is completely unused, because it is not referenced by the spec file.

@kc284
Copy link
Contributor Author

kc284 commented Dec 9, 2016

I've removed it. Now you can resume the review, I've updated the label :)

@kc284
Copy link
Contributor Author

kc284 commented Dec 9, 2016

I think all those targets should have been removed when the SDK VM and the community site went, but they were left behind because it was not obvious what was needed for the docs.

@robhoes
Copy link
Member

robhoes commented Dec 9, 2016

Fantastic ✨

@robhoes robhoes merged commit f1d06bc into xapi-project:master Dec 9, 2016
@kc284 kc284 deleted the master2 branch December 9, 2016 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants