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

Release Candidate API proposal! Breaking changes please read! #854

Closed
davideast opened this issue Mar 12, 2017 · 47 comments
Closed

Release Candidate API proposal! Breaking changes please read! #854

davideast opened this issue Mar 12, 2017 · 47 comments

Comments

@davideast
Copy link
Member

davideast commented Mar 12, 2017

Release Candidate API Proposal

Please read and leave feedback!

AngularFire2 is almost ready for its first semver release! Before that can happen, a few problems must be addressed.

Removing AngularFire for Modularity

AngularFire2 does not take advantage of the Firebase SDK's modularity. Users who only need authentication receive database code and vice versa. The AngularFire service is a central part of this problem. The class includes each feature whether you are using it or not. Even worse, this cannot be tree-shaken. As the library grows to include more features, this will only become more of a problem.

The way to fix this is to remove the AngularFire service and break up the library into smaller @NgModules.

  • AngularFireModule (Firebase app only)
  • AngularFireAuthModule
  • AngularFireDatabaseModule
  • AngularFireStorageModule (Future release)
  • AngularFireMessagingModule (Future release)

This will allow you to get the code for only what features you need. Below is an example setup:

import { NgModule, Component } from '@angular/core';
import { Observable } from 'rxjs/Observable';
import { AngularFireModule } from 'angularfire2';
import { AngularFireDatabaseModule, AngularFireDatabase } from 'angularfire2/database';
import { AngularFireAuthModule, AngularFireAuth } from 'angularfire2/auth';

const config = { }; // firebase config 

@NgModule({
  declarations: [ App ],
  exports: [ App ],
  imports: [ 
    AngularFireModule.initializeApp(config)
    AngularFireDatabaseModule,
    AngularFireAuthModule,
  ],
  bootstrap[ App ]
})
export class MyModule { }

@Component({
  selector: 'my-app',
  template: `
     <div> {{ (items | async)? | json }} </div>
     <div> {{ user.authState | async)? | json }} </div>
  `
})
export class App {
  user: Observable<any>;
  items: Observable<any>;
  constructor(auth: AngularFireAuth, db: AngularFireDatabase) {
     this.items = db.list('items');
  }
}

Instead of pulling each feature off the AngularFire service, you now import each feature module.

Breaking: Simplified Authentication API

The goal of AngularFire (1 & 2) is not to wrap the Firebase SDK. This is something I wrote in a blog post way back in 2014 about AngularFire 0.8.

The goal is to provide Angular-specific functionality. We've done this with FirebaseListObservable and its advanced querying techniques, but the current authentication API offers little outside of the Firebase SDK. The Authentication API also requires the most maintenance as it has quirks across different browsers and platforms like Cordova. Wrapping the API puts AngularFire2 in the middle of these changes, make it prone to break.

My solution is to simplify the API. This, however, means breaking changes. Below is the proposed API and an example:

interface AngularFireAuth {
  auth: FirebaseAuth;
  authState: Observable<firebase.User>;
}
import { NgModule, Component } from '@angular/angularfire2';
import { AngularFireModule } from 'angularfire2';
import { AngularFireAuthModule, AngularFireAuth } from 'angularfire2/auth';

const config = {}; // firebase config

@NgModule({
  declarations: [ App ],
  exports: [ App ],
  imports: [ 
    AngularFireModule.initializeApp(config)
    AngularFireAuthModule,
  ],
  bootstrap[ App ]
})
export class MyModule { }

@Component({
  selector: 'my-app',
  template: ` `
})
export class App {
  constructor(private afAuth: AngularFireAuth) {
    afAuth.authState.subscribe(user => console.log(user));
  }
  signIn() {
    afAuth.auth.signInAnonymously();
  }
}

The authState observable is the best candidate for Angular-specific functionality. The rest of the methods are removed.

This proposal removes custom methods like .createUser(). Most of the custom authentication methods were simply wrapper calls around the official SDK. Rather than creating more bytes for no value, the Firebase Auth instance is available as a property named auth where the same functionality exists.

Remove pre-configure login

This change also removes pre-configuring login. Pre-configuring is a convenient feature, but this API has introduced a fair amount of complexity into the codebase for fairly low value. This is another decision to forgo minor conveniences for simplicity and less code.

I understand that this is likely the hardest of all the breaking changes, so I am open to suggestions. However, I want to keep to the ideals that AngularFire2 is not a wrapper.

We also have plans for authentication based route guards, but more research is needed before we have an API.

Breaking: FirebaseListFactory and FirebaseObjectFactory only take in a Database Reference

This change only affects a few users who directly use FirebaseListFactory. Most people inject AngularFireDatabase directly or use the AngularFire service.

If you directly use FirebaseListFactory you will no longer be able to pass in a string. **The AngularFireDatabase service will still take paths for .list() and .object(). With the FirebaseApp now easily injectable, you can create a reference while still adhering to DI.

This is a minor change, but again it prioritizes simplicity. It keeps the library from writing check logic at multiple abstractions. This again reduces complexity and maintenance.

Below is an example:

import { FirebaseApp } from 'angularfire2';
import { FirebaseListFactory, FirebaseObjectFactory, AngularFireDatabase } from 'angularfire2/database';

@Component({
  selector: 'app',
  template: ``
})
export class App {
  list: Observable<any>;
  constructor(app: FirebaseApp, db: AngularFireDatabase) {
    const listRef = app.database.ref('things');
    const list = FirebaseListFactory(listRef);
    const obj  = FirebaseObjectFactory(listRef.child('thing_1'));
    // const list = FirebaseListFactory('things'); <- no longer valid
    // const obj = FirebaseObjectFactory('things/thing_1'); <- no longer valid
    const list2 = db.list('things'); // still valid
    const obj2 = db.object('things/thing_1'); // still valid
  }
}

Semver: AngularFire2 4.0

Let's talk about naming. AngularFire2 is the second AngularFire. It's not "AngularFire" for Angular 2. Now, I'll admit, this isn't ideal for a name. However, we can't rename the original AngularFire to AngularFireJS. There are many people with AngularFire apps in production and we do not have to cause a problem over vanity.

When we release the semver version will be AngularFire2 4.0.

When the breaking changes are implemented, AngularFire2 will enter version rc0. Once enough adoption and feedback have been received (and bugs fixed), will release to 4.0.

We are investigating scoped packaging to get @angularfire/core, @angularfire/database, @angularfire/auth, and etc. However, there are draw backs to maintaining several scoped packages, such as handling several version mismatches.

Post 4.0

After 4.0 we'll focus on features. Here are the items on the docket:

  • Cloud Storage for Firebase (with a snazzy directive too!)
  • Firebase Cloud Messaging
  • Authentication Route Guards
  • A real documentation site
  • Lazy loading of Firebase Feature Modules (independent of router)
  • Angular Universal Integration

Conclusion: Sticking to a principle

This is a big change. But this change brings several new benefits, and most importantly a guiding principle for AngularFire2. AngularFire2 is not a wrapper around the Firebase SDK. AngularFire2 integrates Firebase into Angular-specific features. If the solution is a simple function wrap, then it doesn't fit the principle.

Your feedback is not only welcome, but it is necessary in making AngularFire2 the best Firebase and Angular experience possible.

@cartant
Copy link
Contributor

cartant commented Mar 12, 2017

Would you also entertain the refactoring of the FirbaseListObservable to expose an underlying observable that makes available added, removed and changed items? It could be a non-breaking change.

When composing RxJS observables, in many situations the FirbaseListObservable is too coarse. The FirbaseListObservable works great with UI stuff, but from my point of view AngularFire2 is also very much about RxJS and I've found that an observable that's inbetween the abstraction of the FirbaseListObservable and the raw SDK child_added, etc. events is a useful thing to have.

Something similar was dismissed in this issue as not adding value over the SDK methods. However, after working with an RxJS/ngrx-based application I don't believe that's the case and AngularFire2's not having an observable that allows working with added, removed and changed items just means that everyone has to write their own. I've had to.

@jsayol
Copy link

jsayol commented Mar 13, 2017

I like this!

Modularity is key and will be a huge improvement in reducing bundle sizes and boot times. Just that is already worth as many breaking changes as needed.

The goal of AngularFire (1 & 2) is not to wrap the Firebase SDK. [...] The goal is to provide Angular-specific functionality.

Agreed. The least the library does, the better. If it's not specific to Angular then it should be done directly via the SDK or through some other library.

About auth, not much to add. Besides wrapping auth state changes in an observable, everything else can be done directly on the SDK. Just some bikesheding: afAuth.authState -> afAuth.state.

About versioning:

When we release the semver version will be AngularFire2 4.0. We will try to keep in line with Angular 4.0 when it releases. I'm open to other suggestions, so drop a comment.

Does that mean maintaining version parity with Angular in the future? Because that wouldn't really be using semver, if AF bumps minor version without any new features, or major versions without actually introducing any breaking changes. If parity is not the goal here, then AngularFire2 3.0 might be more appropriate (I know, I know... more bikeshedding).

As for all the new features planned for the future: yes please, but only if they make sense in an Angular environment. About "Authentication Route Guards": OMG YES.

Last but not least and absolutely least, just an idea: since AF will be split into separate modules, how about publishing them as separate packages instead of as one?

So instead of having just the angularfire2 package and do naked imports from inside it:

import { AngularFireModule, AngularFireApp } from 'angularfire2';
import { AngularFireDatabaseModule, AngularFireDatabase } from 'angularfire2/database';
import { AngularFireAuthModule, AngularFireAuth } from 'angularfire2/auth';

We could have separate packages for each functionality like @angularfire/app, @angularfire/auth, and @angularfire/database like this:

import { AngularFireModule, AngularFireApp } from '@angularfire/app';
import { AngularFireDatabaseModule, AngularFireDatabase } from '@angularfire/database';
import { AngularFireAuthModule, AngularFireAuth } from '@angularfire/auth';

Also... two birds, one stone, right? ;)

@IgorMinar
Copy link

#itJustAngularFire :-)

But seriously: I love the focus on the code size and modularity. I think this is awesome.

What I don't see, is discussion about TypeScript version used to produce the d.ts files and strictNullChecks compatibility. If you are going to cut a major release, then I'd recommend you drop support for older TS and jump on TS 2.2 and make your api surface strictNullChecks compatible.

PS: if you are going break all this stuff, please make sure you document the migration path. These changes while painful are very much necessary, and having good documentation makes the pain hurt less.

PS2: if you are going to break all this stuff, then just rename the thing to angularfire v4.0.0 (drop the "2"). Use version numbers to communicate the difference between v1, v2 and v4. Anything else is very confusing to anyone not closely following the development of the library.

@davideast
Copy link
Member Author

davideast commented Mar 14, 2017

@cartant I'm definitely interested in this idea. If you'd like to create an issue with a proposal, I'd love to discuss it.

@jsayol Upon much thought, I'd like to stick to authState as it wraps the authStateChanged method. Using state can be vague and does not immediately tie the two together. Interested in your thoughts though.

Also I have given serious thought to scoped packaged, I even own the angularfire npm user account. I've been warned by many people smarter than I to avoid scoping packages unless it's completely necessary. I'm still going to do more research, but I do think the time would be now if we were to make that change.

@IgorMinar Thanks for the feedback! I neglected to focus on upgrading TypeScript, but I definitely agree it should be a priority with this release.

Yes there will be a clear migration path. For Database users there will be only small changes. For authentication I will provide the analogous methods in the Firebase SDK.

As for versioning, we can't change the name to angularfire because the AngularFire for AngularJS follows semver which will make the version numbers collide. But I am giving serious though to scoping the packages so we can use @angularfire/core, @angularfire/database, and etc...

@cartant
Copy link
Contributor

cartant commented Mar 14, 2017

@davideast I'm curious as to what the potential problems are with the scoped names. I think that using the @angularfire scope would be the most straightforward way of getting the 2 out of the name.

Hopefully, using multiple, scoped packages won't impose too much of an administrative burden.

@jsayol
Copy link

jsayol commented Mar 14, 2017

@davideast @cartant creating several packages (scoped or not) from a single repo is quite easy using a tool like Lerna. I just went through that process myself and it's straightforward to set up. Those who advised against it, did they give any specific reasons? I'm actually quite interested here.

Just because I gave this some more thought, I'll reiterate: don't jumpt to v4. I think the best course of action would be to:

  1. Publish as scoped packages. This allows to drop the 2 without conflicts.
  2. Follow semver from now on (which you're already planning to do). Even though you could start from scratch with 1.0 on the new packages, versions 1.0 and 2.0 are definitely overloaded right now so it's a good idea to skip them. Skipping 3.0 and jumping to 4.0 only makes sense if AF tracks Angular versions, but like I said earlier that wouldn't really be following semver at all. So, in short, start with 3.0.
  3. Make it clear that, at any point, the latest version of AngularFire (as published with the @angularfire/* packages) is compatible with the latest version of Angular. No need to mention version numbers, and the message is clear.

authState is fine. My main issue with it was that afAuth.authState sounds kind of redundant, but it's true that by itself authState is much more clear than state, which might be too vague.

I'm conflicted about AngularFire exposing the child added/removed/etc as observables. On one hand that's something I always end up doing myself whenever working with Firebase and RxJS so I see some merit as to why it'd be useful to have, but that's not something you might consume directly on the template to update the UI. So this wouldn't really be about Angular but just about providing handy observables to the underlying SDK events, and isn't that the kind of approach the new API is trying to get rid of? I don't feel too strongly about it either way, though, and if it gets added people will definitely use it.

@cartant
Copy link
Contributor

cartant commented Mar 14, 2017

@jsayol @davideast I'll create an issue for the list refactoring sometime soon. It's less about exposing child_added events and more about exposing a more efficient list that doesn't emit everything upon each change. There's a layer in between the the SDK events and the list, and it's inaccessible in the current AngularFire2 and has resulted in my having to replicate code. Hopefully, I'll be able to explain it a little better when I write up the issue.

BTW, I use AngularFire2 extensively, and nowhere do I directly use an AngularFire2 observable in a template.

@WhatsThatItsPat
Copy link
Contributor

If you're worried about semver collisions, just give yourself a big enough buffer that it won't be an issue. Go to plaid and skip all the way to version 10 and you won't be stuck with the numbered angularfire2 name.

I don't think matching Angular at version 4.0 holds any value. Their release schedule will leave AF in the dust soon enough.

That said, I like the look of the scoped package.

@Maistho
Copy link
Contributor

Maistho commented Mar 17, 2017

I like the idea of less code in angularfire2, just to make it easier to follow along with firebase releases, and their docs are a lot more comprehensive than yours.

I do not like the idea of jumping to 4.0 if you're not going to keep version parity with angular. And I don't feel like keeping version parity with angular would give much of a benefit, but it doesn't really matter all too much in the end.

I also think that putting angularfire2 in either @angular/firebase or @angularfire is the best solution. Leaving the 2 in the name will just cause confusion later on.

Compare to @angular/material, their naming/packaging scheme makes perfect sense.

@albanx
Copy link

albanx commented Mar 18, 2017

Can you put also a method that tells if the user is connected or not (not to confuse with the login)? that will be great

@jckeen
Copy link

jckeen commented Mar 21, 2017

I like the idea of breaking up AngularFire2 into smaller modules. The breaking changes do not seem to be like they will be much of a pain as long as you have clear documentation. I think many of us are only Injecting Auth and Database in our services. Removing some of the wrapper functions are also a great idea, it'll force us to learn more from the SDK itself, which is necessary to get a complete user experience.

What I'm really looking forward to is the documentation, specifically best practices. I'd love us to have somewhere to talk about and share exactly how we're using AngularFire2 in order to help develop those practices. It would be nice to get to a point where some of us are sharing modules that we can drop into our projects and build upon. Are you using stores? How are you building them to work with AF? (Just a couple rhetorical questions)

@WebAddict
Copy link

Evolution is happening! Ionic 2 [final] has only been out for 2 months, Angular 4.0 dropped a few weeks ago, and today the first Ionic 3.0 beta was released. There is a clear need for AngularFire to evolve too.

I fully support your proposed changes. I love using AngularFire's bindings for a few specific 'live-data' needs, but most of the data my apps are loading are 'one-time' loads. So most of the time, I'm using firebase's snapshots to pull data.

Please move forward! Thank you!

@cartant
Copy link
Contributor

cartant commented Apr 3, 2017

@albanx If you want an observable that emits a boolean indicating whether or not there is a connection, you can use Firebase's .info:

this.af.database
  .object('.info')
  .map(info => info.connected)
  .subscribe(value => console.log(`connected = ${value}`));

I don't think there's much to be gained by wrapping anything around that.

@elekzalan
Copy link

Can we get an update on this topic, please?

@myspivey
Copy link
Contributor

Just going to throw in my hat of support, this all sounds great. I would like to also see what the effort is to try and stay as current as possible on @angular. angularfire2 is still on 2.0.0 which is causing some issues with the latest cli as mentioned elsehwere and strikes me as though it could be an ongoing problem as @angular cruises through semvar.

DarranShepherd added a commit to DarranShepherd/githyde that referenced this issue Apr 17, 2017
Redirect was only sending the user:email scope to GitHub even when requesting repo. As per angular/angularfire#798 this does not happen when using popup. However, the accessToken isn't visible in the observable of auth states, but is in the promise returned from login.
Given the impending RC of AngularFire2 will require reworking of the auth code (see angular/angularfire#854), this will do for now.
@davideast
Copy link
Member Author

davideast commented Apr 17, 2017

@elekzalan @myspivey The first next release is available for testing and it supports Angular 4.

npm i angularfire2@next --save

@myspivey
Copy link
Contributor

Loaded and so far so good! Did not have Auth in this simple little POC so unable to test that but the breakout is great, simplified our code a bit. Great work! Will report back if I run into any issues.

@Maistho
Copy link
Contributor

Maistho commented Apr 18, 2017

Tried migration my smallish app that uses db and Auth, and apart from spending some time reworking the Authentication (it's much easier to link accounts now) it was completely painless. Looking great!

@Shyam-Chen
Copy link

Shyam-Chen commented Apr 27, 2017

// Md => Material design
import {
  MdButtonModule,
  MdCheckboxModule,
  MdSelectModule,
  MdInputModule,
  MdSnackBarModule
} from '@angular/material';

// Scope: `@angular`
import { AngularFireAppModule } from '@angular/firebase-app';
import { AngularFireDatabaseModule } from '@angular/firebase-database';
import { AngularFireAuthModule } from '@angular/firebase-auth';

// Fa => Firebase application
import {
  FaAppModule,
  FaDatabaseModule, Database, ListFactory
  FaAuthModule, Auth, AuthProviders
} from '@angular/firebase';  // Can it be tree-shaking?

@almothafar
Copy link

Regarding the version, AngularFire2 maybe can be AngularFireX if you can't rename AngularFire to AngularFireJS, at least for now, one step then you can maybe after a while to rename the AngularFire to JS version.

I see most of the modules did that, it will make sense for now I think.

BTW, the rc0 version is really nice, I feel that while I was migrating :)

@s3ppo
Copy link

s3ppo commented Apr 29, 2017

anybody can give me an example how to do the googleprovider or facebookprovider login now? thank you :)

@Maistho
Copy link
Contributor

Maistho commented Apr 29, 2017

@s3ppo just follow the firebase docs but replace firebase.auth() with the AngularFireAuth.auth object.

constructor(private fbAuth: AngularFireAuth) {
  this.fbAuth.auth.signInWithPopup(new firebase.auth.GoogleAuthProvider());
}

@s3ppo
Copy link

s3ppo commented Apr 29, 2017

@Maistho
thanks, that looks easy :)

@rodrifmed
Copy link

@s3ppo You have to use FirebaseApp;

like this example:

storage :any;
constructor(private app: FirebaseApp) {
      this.storage = app.storage().ref();
}

@exxmen
Copy link

exxmen commented May 3, 2017

getting No Provider for AngularFireAuth during run time.. not sure what i missed. added authservice in the app module providers

@almothafar
Copy link

@exxmen you need to import:

    AngularFireModule.initializeApp(config)
    AngularFireAuthModule,

inside your module, for instance, app.module.ts

@GunnarGitHub
Copy link

Hi,
I've upgraded to firebase2/database.
I get the following problem. How can I fix?
Error: No provider for AngularFireDatabase!
Gunnar

@almothafar
Copy link

@GunnarGitHub

You need to import AngularFireDatabaseModule as well after AngularFireModule.initializeApp(config) inside your module.

fisherds added a commit to fisherds/angularfire2 that referenced this issue May 4, 2017
Copied some material from angular#854 into this doc
@exxmen
Copy link

exxmen commented May 4, 2017

thanks @almothafar also had to include all the imported ionic-natives (Facebook, GooglePlus, etc.) to the providers in appmodule.

davideast pushed a commit that referenced this issue May 4, 2017
* Add information about individual @ngModules

Copied some material from #854 into this doc

* add comments based on feedback
@davideast
Copy link
Member Author

It has been merged and released!

@damgero
Copy link

damgero commented May 6, 2017

Just updated to this, was easy as cake, and works like a charm!, thanks for your great job!

@harshavarun25
Copy link

@davideast I am having a similar problem can you please help me fix. I am like struggling because of this.

[23:00:27] typescript: C:/Users/Harsha Varun/MyIonicProject/src/pages/third/third.ts, line: 4
Module '"C:/Users/Harsha Varun/MyIonicProject/node_modules/angularfire2/index"' has no exported member
'AuthProviders'.

L3: import {FourthPage} from '../fourth/fourth';
L4: import { AuthProviders, AuthMethods, AngularFire } from 'angularfire2';
[23:00:27] typescript: C:/Users/Harsha Varun/MyIonicProject/src/pages/third/third.ts, line: 4
Module '"C:/Users/Harsha Varun/MyIonicProject/node_modules/angularfire2/index"' has no exported member
'AuthMethods'.

L3: import {FourthPage} from '../fourth/fourth';
L4: import { AuthProviders, AuthMethods, AngularFire } from 'angularfire2';
[23:00:27] typescript: C:/Users/Harsha Varun/MyIonicProject/src/pages/third/third.ts, line: 4
Module '"C:/Users/Harsha Varun/MyIonicProject/node_modules/angularfire2/index"' has no exported member
'AngularFire'.

L3: import {FourthPage} from '../fourth/fourth';
L4: import { AuthProviders, AuthMethods, AngularFire } from 'angularfire2';
package.json
"name": "MyIonicProject",
"version": "0.0.1",
"author": "Ionic Framework",
"homepage": "http://ionicframework.com/",
"private": true,
"scripts": {
"clean": "ionic-app-scripts clean",
"build": "ionic-app-scripts build",
"lint": "ionic-app-scripts lint",
"ionic:build": "ionic-app-scripts build",
"ionic:serve": "ionic-app-scripts serve"
},
"dependencies": {
"@angular/common": "4.1.0",
"@angular/compiler": "4.1.0",
"@angular/compiler-cli": "4.1.0",
"@angular/core": "4.1.0",
"@angular/forms": "4.1.0",
"@angular/http": "4.1.0",
"@angular/platform-browser": "4.1.0",
"@angular/platform-browser-dynamic": "4.1.0",
"": "3.7.0",
"": "3.7.0",
"": "3.7.0",
"": "2.0.1",
"angularfire2": "^4.0.0-rc.0",
"firebase": "^4.0.0",
"ionic-angular": "3.2.1",
"ionicons": "3.0.0",
"rxjs": "5.1.1",
"sw-toolbox": "3.6.0",
"zone.js": "0.8.10"
},
"devDependencies": {
"": "1.3.7",
"": "1.1.2",
"": "1.1.2",
"angularfire2": "^4.0.0-rc.0",
"typescript": "2.2.1"
},
"description": "An Ionic project"
}

Thanks in advance

@almothafar
Copy link

@harshavarun25 you really should read migration steps, also, you reached this issue, you can find the solution with comments as well.

@WhatsThatItsPat
Copy link
Contributor

@harshavarun25 read the migration & docs, as @almothafar said.

But on top of that, don't spam the repo in three--separate--places.

@Toxicable
Copy link

@raugaral This is the wrong place to ask that question.
Also please don't spam the repo, you asked your question in 3 separate issues, this is unnecessary

starglow-ventures pushed a commit to starglow-ventures/Angular-Firebase that referenced this issue Jul 3, 2018
* Add information about individual @ngModules

Copied some material from angular/angularfire#854 into this doc

* add comments based on feedback
noproblem23 added a commit to noproblem23/angularfire that referenced this issue Apr 26, 2023
* Add information about individual @ngModules

Copied some material from angular/angularfire#854 into this doc

* add comments based on feedback
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