Skip to content

Documenting the Console Helpers #1999

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

Merged
merged 7 commits into from
Dec 26, 2012
Merged

Documenting the Console Helpers #1999

merged 7 commits into from
Dec 26, 2012

Conversation

Sgoettschkes
Copy link
Contributor

As suggested in #1811, this PR includes documention on the two Console Helpers:

  • DialogHelper
  • FormatterHelper

The new documents are included in the index and checked for sphinx issues.

@wouterj
Copy link
Member

wouterj commented Dec 4, 2012

GitHub don't let me create a PR on your repo, but you should change this format issues: wouterj/symfony-docs@69ce6c9

@Sgoettschkes
Copy link
Contributor Author

@wouterj Done. I think to be able to make a PR you would need to add my repository as a new remote, pull my branch and make changes. Maybe there is a better way like giving you push access to my repository or you giving me push access.

@wouterj
Copy link
Member

wouterj commented Dec 5, 2012

@Sgoettschkes well, I added your repo as a remote and have based my commit on your branch. But in order to create a PR on github you should choose a branch in the dropdown list and it looks like Github doesn't refresh it if the main repo is forked by a new user. So your repo was not in the list of repo's to request a PR and I wasn't able to add you in there...

@Sgoettschkes
Copy link
Contributor Author

I would love to see this merged so we can get started documenting the new ProgressHelper and new DialogHelper capabilities.

Any change requests for this PR?

@wouterj
Copy link
Member

wouterj commented Dec 22, 2012

@Sgoettschkes 👍, but be aware that the ProgressHelper is already documented (it is included in the introduction chapter right now, it should be copied into a new file inside the helper dir)

@weaverryan
Copy link
Member

Hi Sebastian!

Sorry for the delay on merging this - it's a really great PR! I've now merged it in and made only a few minor changes.

As @wouterj said, the ProgressHelper is documented already, but I much prefer your approach of placing these helpers in their own documents, instead of inside on big document. Could you move the ProgressHelper docs into the new format? I'll merge everything up to master now so that the new code is available there.

Thanks!

@wouterj
Copy link
Member

wouterj commented Dec 26, 2012

@Sgoettschkes are you going to document these new helpers/features? (otherwise I will d it)

@Sgoettschkes
Copy link
Contributor Author

As said I'll do it. I'll also take care of adding the new features as menttioned in #2022!

@Sgoettschkes Sgoettschkes deleted the issue1811 branch December 26, 2012 19:15
@wouterj
Copy link
Member

wouterj commented Dec 26, 2012

ok, great job! (almost all components are documented right now 🚴)

weaverryan added a commit that referenced this pull request Dec 26, 2012
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.

3 participants