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

Fixes #132: option added to push documentation to GH-PAGES #133

Merged
merged 1 commit into from
Jun 17, 2017

Conversation

poonai
Copy link
Contributor

@poonai poonai commented Jun 16, 2017

Description

option added to push documentation to GH-PAGES

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

  • Bug fix
  • New feature

Checklist:

  • I have read the Contribution & Best practices Guide and my PR follows them.
  • There is a corresponding issue for this pull request.
  • Mentioned the Issue number in the pull request commit message Fixes #<number> commit message
  • There is only one commit per issue.

@poonai
Copy link
Contributor Author

poonai commented Jun 16, 2017

@pri22296 @imujjwal96 please check the implementation.

@pri22296
Copy link
Member

@Sch00lb0y please fix codacy issues

ghdeploy.sh Outdated
@@ -0,0 +1,45 @@
#!/bin/bash
Copy link
Member

Choose a reason for hiding this comment

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

Instead of creating this script, use the existing publish_docs.sh otherwise it would again lead to situations like #108 . ensure that the script is usable from both a webUI and travis. use idea similar to generate.sh to run conditional code based on whether running from a webUI.

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 have made changes

@imujjwal96
Copy link
Contributor

@Sch00lb0y Can we see a deployment for this?

@poonai poonai force-pushed the oauth branch 2 times, most recently from 7aabe3b to 0fa8d45 Compare June 16, 2017 18:34
@poonai
Copy link
Contributor Author

poonai commented Jun 16, 2017

@imujjwal96 demo link

@poonai poonai changed the title [WIP] Fixes #132: option added to push documentation to GH-PAGES Fixes #132: option added to push documentation to GH-PAGES Jun 16, 2017
Copy link
Member

@pri22296 pri22296 left a comment

Choose a reason for hiding this comment

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

Also why does this require permission to delete my repository?

publish_docs.sh Outdated
echo -e "DOCURL not set. Using default github pages URL"
else
echo $DOCURL > CNAME
if ![ "${WEBUI:-false}" == "true" ]; then
Copy link
Member

Choose a reason for hiding this comment

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

why was this change made?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

now WebUI don't have CNAME. I'll remove the condition

Copy link
Member

Choose a reason for hiding this comment

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

yeah but users can still specify the docurl in the .yaydoc.yml file.

var donePercent = 0;
var projectName = data.gitURL.split("/")[4].split(".")[0];
var gitURL = "http://" + data.username+":" + crypter.decrypt(data.encryptedToken) + "@github.com/" + data.username + "/" + projectName;
Copy link
Member

Choose a reason for hiding this comment

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

Instead of building the url here, you could have passed the token as OAUTH_TOKEN and the publish script should have handled that.

@poonai
Copy link
Contributor Author

poonai commented Jun 16, 2017

@pri22296 for pushing repo have to get the access.even in open-event-webapp if users want to push, they requesting delete level access

@pri22296
Copy link
Member

@Sch00lb0y If that's the case, then let it be

@@ -1,15 +1,13 @@
var exports = module.exports = {};
Copy link
Contributor

Choose a reason for hiding this comment

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

why did you remove this?

};

exports.deployPages = function (socket, data) {
Copy link
Contributor

Choose a reason for hiding this comment

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

generator.js is the backend script for documentation generation. Move this function to a separate file.

@@ -44,6 +44,7 @@ $(function () {
$('#notification-container').css("visibility", "hidden");
$('#notification-container').css("opacity", "0");
}, 5000)
$("#buttons > .row").append('<a href="/github?email='+data.email+'&uniqueId='+data.uniqueId+'&gitURL='+data.gitUrl+'" class="btn btn-default" id="btnDeploy">deploy</a>')
Copy link
Contributor

Choose a reason for hiding this comment

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

Please follow the same pattern as above

@imujjwal96
Copy link
Contributor

The link is not working

@poonai poonai force-pushed the oauth branch 4 times, most recently from 92d77b3 to d899dc5 Compare June 17, 2017 09:19
@poonai
Copy link
Contributor Author

poonai commented Jun 17, 2017

@pri22296 @imujjwal96 I have made changes. here is the new link http://yaydoc.pagupu.in please check

@imujjwal96
Copy link
Contributor

Although the documentation was pushed successfully, the progress bar is not completed.
image

@poonai
Copy link
Contributor Author

poonai commented Jun 17, 2017

@imujjwal96 I have fixed please check now

@imujjwal96
Copy link
Contributor

imujjwal96 commented Jun 17, 2017

The reason why webapp requires Delete permission
Transcript from open-event-webapp gitter chat
image

@poonai
Copy link
Contributor Author

poonai commented Jun 17, 2017

@imujjwal96 i have updated the permission scope

views/index.jade Outdated
@@ -35,6 +35,7 @@ block content
.row
a.btn.btn-default(type='button' id='btnDownload') Download
a.btn.btn-default(type='button' id='btnPreview' target='_blank') Preview
a.btn.btn-default(type='button' id='btnDeploy') deploy
Copy link
Contributor

Choose a reason for hiding this comment

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

Deploy :)

Copy link
Contributor

@imujjwal96 imujjwal96 left a comment

Choose a reason for hiding this comment

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

Looks good. Nice job @Sch00lb0y

@pri22296
Copy link
Member

pri22296 commented Jun 17, 2017

@Sch00lb0y any specific reason for not reusing the existing console for ghpages deploy?

@poonai
Copy link
Contributor Author

poonai commented Jun 17, 2017

@pri22296 If we keep on adding things to one page. it'll be hard to maintain and also I'm templating the deploy page according to the need. In open-event-webapp also they maintaining separate page for deploying

@mariobehling mariobehling merged commit dc18c38 into fossasia:master Jun 17, 2017
@poonai poonai deleted the oauth branch June 18, 2017 09:19
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.

None yet

4 participants