Adding "View Source" link to the class page #91
Conversation
|
@fabpot , the code style check is reporting issues that existed in changed files, but not in the code changed in this PR. For example Scrutinizer CI remembers what issues exists in |
|
I've rebased on master so now it can be merged from GitHub website. |
|
I've fixed CS issues reported by status.fabpot.io. I've kept is a separate commit, because CS violations weren't introduced by my changes. |
|
|
||
| public function __construct($name, $localPath) | ||
| { | ||
| $this->name = $name; |
There was a problem hiding this comment.
name is not used by the AbstractRemoteRepository so it should be moved to the GitHubRemoteRepository
There was a problem hiding this comment.
This is because I planned to create also BitBucketRemoteRepository, but wasn't able to do so due some stupid link building rules there where a link to a file always has latest commit sha, when that file was changed. Since we don't track commit sha associated with version/file, then it's impossible to build such links.
In general I would say that on all remote repositories a repository is identified by name, so having this property in parent abstract class made more sense to me.
|
@fabpot , any opinion on this PR. Is it needed. If so, then what should I adjust for it to be merged? |
|
@fabpot , ping. Please see #91 (comment) . |
|
Was anything like this PR ever integrated into Sami? |
|
@fabpot , another ping. If you need anything fixed, then I can do that. Otherwise if nobody has any objections, then please merge. |
| @@ -99,6 +102,7 @@ And here is how you can configure different versions: | |||
| 'title' => 'Symfony2 API', | |||
| 'build_dir' => __DIR__.'/../build/sf2/%version%', | |||
| 'cache_dir' => __DIR__.'/../cache/sf2/%version%', | |||
| 'remote_repository' => new GitHubRemoteRepository('symfony/symfony', dirname($dir)), | |||
There was a problem hiding this comment.
This should probably be documented as an additional optional feature. Here, people might think this is required.
There was a problem hiding this comment.
The comment above says The``Sami``constructor optionally takes an array of options as a second argument: which kind of implies that all options are optional. Any ideas on how we can stress that fact even more, it's even needed?
|
This PR looks good to me. |
aik099
force-pushed
the
63-view-source-link-feat
branch
from
fe5a59a
to
77c2386
Compare
aik099
force-pushed
the
63-view-source-link-feat
branch
from
77c2386
to
6547fec
Compare
1. refactoring to increase clarity of added logic 2. adding parenthesis around "View Source" link 3. updating documentation 4. adding "GitHubRemoteRepository" class import to the examples
aik099
force-pushed
the
63-view-source-link-feat
branch
from
6547fec
to
a077a75
Compare
|
I've squashed and rebased PR so now it can be merged. The failure of
|
|
Thank you @aik099. |
…099) This PR was merged into the 3.0-dev branch. Discussion ---------- Adding "View Source" link to the class page This is an implementation of #63 task based on original commit made in #64. ___Usage___ By default Sami doesn't know on which hosting Git repository is located and doesn't add `View Source` links on class detail pages. However if you specify `remote_repository` option with instance of `AbstractRemoteRepository` class sub-class, then the `View Source` link would be added:  For example: ```php array( ... 'remote_repository' => new GitHubRemoteRepository('username/repository', '/path/to/repository'), ... ) ``` The `'/path/to/repository'` is required, because Sami doesn't know base folder on GitHub (or any other service), where original files (for which documentation is being generated) live. Thanks to @anlutro, which commit (squashed) was used as a base for my changes. Closes #63 Commits ------- a077a75 Extracting url building logic to dedicated GitHubRemoteRepository class 21dcc46 Working prototype with class "View Source" links
|
Thanks for merging, but apparently the Packagist hook doesn't work because it still referencing old commit for 3.0.0-DEV version. |
|
This is a new feature. Don't we need to bump the minor version now? |
The link will be shown as the "at line ###" part for each method. Using the link, the users can directly jump to the method source. This is an extension of FriendsOfPHP#91
…nfigured (mzur) This PR was merged into the 3.0-dev branch. Discussion ---------- Add a link to class methods if a remote repository is configured The link will be shown as the "at line ###" part for each method.  Using the link, the users can directly jump to the method source. This is an extension of #91 Commits ------- 6bac774 Add a link to class methods if a remote repository is configured
|
For anyone interested I brewed a solution to generate viewable source code along the API documentation, using GeSHi. |
This is an implementation of #63 task based on original commit made in #64.
_Usage_
By default Sami doesn't know on which hosting Git repository is located and doesn't add
View Sourcelinks on class detail pages. However if you specifyremote_repositoryoption with instance ofAbstractRemoteRepositoryclass sub-class, then theView Sourcelink would be added:For example:
The
'/path/to/repository'is required, because Sami doesn't know base folder on GitHub (or any other service), where original files (for which documentation is being generated) live.Thanks to @anlutro, which commit (squashed) was used as a base for my changes.
Closes #63