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

New branch 'dev' for developpement #9

Closed
Sbgodin opened this issue Jul 30, 2014 · 6 comments
Closed

New branch 'dev' for developpement #9

Sbgodin opened this issue Jul 30, 2014 · 6 comments
Assignees

Comments

@Sbgodin
Copy link

Sbgodin commented Jul 30, 2014

Hi all!

I integrated my patch titleLink in a new branch dev. I suggest that we make all developments in the branch dev. The stable would remain in master. Before releasing a new stable, we may discuss/vote before merging to master and then tag the new release.

Please let me now if all this fits to you.

@Sbgodin Sbgodin self-assigned this Jul 30, 2014
@e2jk
Copy link

e2jk commented Jul 30, 2014

I would prefer using the topic-branch workflows:

You decide to fix a bug/enhancement. Great!

  • [one time setup] Fork Shaarli's repository (shaarli/Shaarli) to your own user in Github. Clone your repository to your development machine.
  • Update your master branch with shaarli/Shaarli 's master branch, to make sure you are up to date with all the latest changes from the project before starting your development.
  • Create a local topic branch (a branch named e.g. "<ticket ID>-<short descriptive name>") and make your changes to that branch.
  • Push the changes to your fork on Github.
  • Create a pull request from your topic branch to the master branch on shaarli/Shaarli
  • Other folks on the project (can also be from folks that don't [yet] have commit access to the main repo) review your pull request, after an explicit OK on the pull request ticket either you or the person approving your change merges the code changes from your topic branch to the master branch.

I prefer to have this "review, then commit to master" workflow instead of the dev branch, as merging from dev to master would be a larger task to be done prior to releasing a new version, and having the changes directly in master allows for easier testing before a release. This means that we need to keep master always in a working state, hence the mandatory review of the pull request.

Don't make changes to your local Master branch, otherwise you'll have trouble updating your Master branch from shaarli/Shaarli 's master branch the next time you want to work on a bug/enhancement. This is really to make your life easier ;)

I would suggest one exception to the "always have an explicit OK on your pull request": if you're fixing trivial stuff, such as fixing a typo, removing whitespace, etc., go ahead and push your change directly to master.

Let me know what workflow you'd like to use. Regardless of what is chosen, we'll have to document that on the wiki for new contributors to see (that will be the close criteria for this issue)

@nodiscc
Copy link
Member

nodiscc commented Jul 30, 2014

It looks ok but would require bumping the version number in index.php for every change in master (as discussed in #5) to make sure users have/are aware of the latest minor version and fixes.

My policy would be: never touch master unless we have a bugfix or a few new features to release. If all the work is done in dev it should be easy to merge it/rebase on top of master then merge for new releases. Consider dev branch as a sort of beta release channel.

Or we can use topic branches and merge directly in master, but that means more releases/less time to fix if something goes wrong. Your call.

@e2jk
Copy link

e2jk commented Jul 30, 2014

To solve the issue of the version number in index.php (which is, by the way, an issue already, nothing new) I would suggest the following:

  • Version 0.0.42beta is released.
  • The very next commit on master updates the version string in index.php to be "0.0.43beta-dev", and stays that way until 0.0.43beta is ready for release.
  • The last commit before releasing that new version updates the version string to be "0.0.43beta" (i.e. remove "-dev".

That would just be part of the release process, similar to tagging a specific commit.
So if you're running Shaarli from an official release tarball, you have a version that doesn't end in -dev.
if you did a git clone, you'll see "-dev" in your version string.

I'm worried that the dev branch won't get tested at all, and that by consequence we might only find issues when they are already part of an officially released version, instead of finding that out because a brave soul decided to use the bleeding edge from master and helped us find a bug.
We're a small project, we can use all the help we get ;)
Also, making a release would be more complex if you need to go through all the changes in the dev branch, to make sure none gets forgotten. And how will you decide if something should go from dev to master, if the original pull request is already closed (from topic branch to dev).

Have a look at the jQuery workflow, which is pretty much what I described in my post above.

All being said, in my role as Debian maintainer I am primarily interested in the official tarballs, how we get to those is not that critical, as long as we get our bugs sorted out in time and we don't forget to merge any pull requests ;)

@Sbgodin
Copy link
Author

Sbgodin commented Jul 30, 2014

@e2jk I agree with almost everything. I just use the dev branch as you do for the master, which will only be downloaded by people who wants stable version. About bug tracking in the dev, all the developers and interested people should use the dev branch. Anyway, I don't see any major concern using master directly.

About the revision number, if we use master/dev schema, the "-dev" is removed upon merge. So there's no curious changei (add/del -dev) in the dev branch.

As @nodiscc said, still in the master/dev schema, the master branch is stable. So, only hotfixes should get there, for a security issue for instance. Without a dev branch, this hotfixe could interfere with the current developments.

I'll adapt to any workflow.

@nodiscc
Copy link
Member

nodiscc commented Jul 30, 2014

Reading what @e2jk said, the jQuery-style workflow may be good enough, and simpler. Just test feature branches thoroughly before they are merged to master.

@Sbgodin
Copy link
Author

Sbgodin commented Jul 30, 2014

Okay for me. I deleted the dev branch, and replaced it by the master branch.

@nodiscc nodiscc closed this as completed Aug 3, 2014
ajabep added a commit to ajabep/Shaarli that referenced this issue Apr 2, 2021
When we try to access the atom feed and have no bookmarks, it raised the following exception :

```
Call to a member function reorder() on array /webroot/application/bookmark/BookmarkFileService.php:143
#0 /webroot/application/feed/FeedBuilder.php(106): Shaarli\Bookmark\BookmarkFileService->search(Array, 'public', false, false, true)
shaarli#1 /webroot/application/front/controller/visitor/FeedController.php(47): Shaarli\Feed\FeedBuilder->buildData('atom', Array)
shaarli#2 /webroot/application/front/controller/visitor/FeedController.php(20): Shaarli\Front\Controller\Visitor\FeedController->processRequest('atom', Object(Slim\Http\Request), Object(Slim\Http\Response))
shaarli#3 [internal function]: Shaarli\Front\Controller\Visitor\FeedController->atom(Object(Slim\Http\Request), Object(Slim\Http\Response), Array)
shaarli#4 /webroot/vendor/slim/slim/Slim/Handlers/Strategies/RequestResponse.php(40): call_user_func(Array, Object(Slim\Http\Request), Object(Slim\Http\Response), Array)
shaarli#5 /webroot/vendor/slim/slim/Slim/Route.php(281): Slim\Handlers\Strategies\RequestResponse->__invoke(Array, Object(Slim\Http\Request), Object(Slim\Http\Response), Array)
shaarli#6 /webroot/application/front/ShaarliMiddleware.php(55): Slim\Route->__invoke(Object(Slim\Http\Request), Object(Slim\Http\Response))
shaarli#7 [internal function]: Shaarli\Front\ShaarliMiddleware->__invoke(Object(Slim\Http\Request), Object(Slim\Http\Response), Object(Slim\Route))
shaarli#8 /webroot/vendor/slim/slim/Slim/DeferredCallable.php(57): call_user_func_array(Array, Array)
shaarli#9 [internal function]: Slim\DeferredCallable->__invoke(Object(Slim\Http\Request), Object(Slim\Http\Response), Object(Slim\Route))
shaarli#10 /webroot/vendor/slim/slim/Slim/MiddlewareAwareTrait.php(70): call_user_func(Object(Slim\DeferredCallable), Object(Slim\Http\Request), Object(Slim\Http\Response), Object(Slim\Route))
shaarli#11 /webroot/vendor/slim/slim/Slim/MiddlewareAwareTrait.php(117): Slim\Route->Slim\{closure}(Object(Slim\Http\Request), Object(Slim\Http\Response))
shaarli#12 /webroot/vendor/slim/slim/Slim/Route.php(268): Slim\Route->callMiddlewareStack(Object(Slim\Http\Request), Object(Slim\Http\Response))
shaarli#13 /webroot/vendor/slim/slim/Slim/App.php(503): Slim\Route->run(Object(Slim\Http\Request), Object(Slim\Http\Response))
shaarli#14 /webroot/vendor/slim/slim/Slim/MiddlewareAwareTrait.php(117): Slim\App->__invoke(Object(Slim\Http\Request), Object(Slim\Http\Response))
shaarli#15 /webroot/vendor/slim/slim/Slim/App.php(392): Slim\App->callMiddlewareStack(Object(Slim\Http\Request), Object(Slim\Http\Response))
shaarli#16 /webroot/vendor/slim/slim/Slim/App.php(297): Slim\App->process(Object(Slim\Http\Request), Object(Slim\Http\Response))
shaarli#17 /webroot/index.php(177): Slim\App->run(true)
shaarli#18 {main}
```
ArthurHoaro pushed a commit to ArthurHoaro/Shaarli that referenced this issue May 8, 2021
When we try to access the atom feed and have no bookmarks, it raised the following exception :

```
Call to a member function reorder() on array /webroot/application/bookmark/BookmarkFileService.php:143
#0 /webroot/application/feed/FeedBuilder.php(106): Shaarli\Bookmark\BookmarkFileService->search(Array, 'public', false, false, true)
#1 /webroot/application/front/controller/visitor/FeedController.php(47): Shaarli\Feed\FeedBuilder->buildData('atom', Array)
#2 /webroot/application/front/controller/visitor/FeedController.php(20): Shaarli\Front\Controller\Visitor\FeedController->processRequest('atom', Object(Slim\Http\Request), Object(Slim\Http\Response))
#3 [internal function]: Shaarli\Front\Controller\Visitor\FeedController->atom(Object(Slim\Http\Request), Object(Slim\Http\Response), Array)
#4 /webroot/vendor/slim/slim/Slim/Handlers/Strategies/RequestResponse.php(40): call_user_func(Array, Object(Slim\Http\Request), Object(Slim\Http\Response), Array)
#5 /webroot/vendor/slim/slim/Slim/Route.php(281): Slim\Handlers\Strategies\RequestResponse->__invoke(Array, Object(Slim\Http\Request), Object(Slim\Http\Response), Array)
shaarli#6 /webroot/application/front/ShaarliMiddleware.php(55): Slim\Route->__invoke(Object(Slim\Http\Request), Object(Slim\Http\Response))
shaarli#7 [internal function]: Shaarli\Front\ShaarliMiddleware->__invoke(Object(Slim\Http\Request), Object(Slim\Http\Response), Object(Slim\Route))
shaarli#8 /webroot/vendor/slim/slim/Slim/DeferredCallable.php(57): call_user_func_array(Array, Array)
shaarli#9 [internal function]: Slim\DeferredCallable->__invoke(Object(Slim\Http\Request), Object(Slim\Http\Response), Object(Slim\Route))
shaarli#10 /webroot/vendor/slim/slim/Slim/MiddlewareAwareTrait.php(70): call_user_func(Object(Slim\DeferredCallable), Object(Slim\Http\Request), Object(Slim\Http\Response), Object(Slim\Route))
shaarli#11 /webroot/vendor/slim/slim/Slim/MiddlewareAwareTrait.php(117): Slim\Route->Slim\{closure}(Object(Slim\Http\Request), Object(Slim\Http\Response))
shaarli#12 /webroot/vendor/slim/slim/Slim/Route.php(268): Slim\Route->callMiddlewareStack(Object(Slim\Http\Request), Object(Slim\Http\Response))
shaarli#13 /webroot/vendor/slim/slim/Slim/App.php(503): Slim\Route->run(Object(Slim\Http\Request), Object(Slim\Http\Response))
shaarli#14 /webroot/vendor/slim/slim/Slim/MiddlewareAwareTrait.php(117): Slim\App->__invoke(Object(Slim\Http\Request), Object(Slim\Http\Response))
shaarli#15 /webroot/vendor/slim/slim/Slim/App.php(392): Slim\App->callMiddlewareStack(Object(Slim\Http\Request), Object(Slim\Http\Response))
shaarli#16 /webroot/vendor/slim/slim/Slim/App.php(297): Slim\App->process(Object(Slim\Http\Request), Object(Slim\Http\Response))
shaarli#17 /webroot/index.php(177): Slim\App->run(true)
shaarli#18 {main}
```
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

No branches or pull requests

3 participants