Build and plugin hooks refactoring for code splitting and asset emission workflows #2208

guybedford · 2018-05-22T23:41:00Z

This is the rework of #2126.

Resolves #2163, #2092, #1611, #1629, #1282, #803.

Refactoring Summary (for the third time...):

transformBundle has been renamed to transformChunk taking one extra argument - the chunk itself
I've added buildStart and buildEnd hooks, that apply for the rollup.rollup lifecycle, regardless of generate or write calls. buildEnd takes an error parameter if there was a build error.
The plugin context is extended to include this.resolveId and this.isExternal, as well as the existing this.warn and this.error and now applies equally for all plugin hooks
entryNames and chunkNames are now entryFileNames and chunkFileNames.
Code splitting is refactored into a single path based on a inlineDynamicImports input option, which by default is set to try for a single string input and false otherwise

Asset Emission Features

The goal here is to support workflows of building assets like CSS alongside chunks.

Use cases such as:

Being able to work out only which CSS is associated with a given chunk
Being able to get the URL of an asset in a straightfoward way from within module code
Being able to tell which exports have been tree-shaken from a module to prune CSS (eg treeshaking for CSS modules)
Being able to run in-memory builds of chunks and assets and get both of their sources without having to write the files - useful for server side rendering style workflows

I've really gone back and fourth on a lot of these quite a bit to try and have Rollup do the minimal work necessary to keep maintenance down and to just open the right doors.

This PR provides:

A new assetFileNames option is added, which by default is set to assets/[name]-[hash][ext].
Plugin context provides emitAsset(name: string, source?: string | Buffer) => assetId: string, setAssetSource(assetId: string, source: string | Buffer) and getAssetFileName(assetId: string). This way an asset can be defined by a plugin at any time, and assets can also have their sources deferred until after the chunking and treeshaking so that they can perform optimizations based on matching the chunking and treeshaking behaviours. getAssetFileName can only be called after both the asset source is set and generate has been called, throwing an error otherwise.
A special token import.meta.ROLLUP_ASSET_URL_[assetid] is supported in code which is replaced in the build just like import.meta.url but instead pointing to the URL of the given asset. Getting the AssetID early here is important to be able to emit these tokens in say translate, while the asset source itself might only be populated afterwards.
A generateBundle(outputOptions, outputBundle, isWrite) hook is provided ideally to replace onwrite and ongenerate in the deprecation path. The outputBundle takes the form of { [fileName: string]: Chunk | string | Buffer }, where a string or buffer indicates an asset.
The Chunk.modules list is updated to no longer be an array of module Ids, but an object of the form { [moduleId: string]: { renderedExports: string[], removedExports: string[], renderedLength: number, originalLength: number } }. This way it is possible to determine what exports have been tree-shaken, as well as workout out how much source reduction that corresponds to.
rollup.generate and rollup.write still return the direct chunk in the single-file build case (being deprecated), and return the outputBundle object now by default when experimentalCodeSplitting is enabled.

I think the above finally gets us these use cases in a relatively simple and straightforward way for Rollup, allowing plugins to explore the space further and drawing a clear line on some of these hard problems so that we don't fall too deep into the rabbit hole on these things.

@lukastaegert ready for review finally :)

lukastaegert

Just a tiny beginning of a review, looks really promising, great work so far!

lukastaegert · 2018-05-25T16:24:56Z

src/rollup/types.d.ts

@@ -265,10 +322,10 @@ export interface RollupCache {
 	modules: ModuleJSON[];
 }

-export interface Bundle {
+export interface RollupFileBuild {


This name confused me at first as we are always building files. Maybe RollupSingleFileBuild as the "single" part is the important one ?

lukastaegert · 2018-05-25T16:27:47Z

src/ast/nodes/MemberExpression.ts

-				this.property instanceof Identifier && this.property.name,
-				options.format
-			);
-			if (importMetaMechanism) code.overwrite(this.start, this.end, importMetaMechanism);


I think we already touched this in one of the previous reviews but great this could be moved to MetaProperty!

lukastaegert · 2018-05-25T16:31:12Z

src/finalisers/amd.ts

@@ -16,7 +16,7 @@ export default function amd(
 		intro,
 		outro,
 		dynamicImport,
-		importMeta,
+		hasImportMetaUrl,


lukastaegert

So this is the next round of reviews. This time, I focused on trying out as much as possible though I did not manage to test everything yet.

Overall this is really impressive and seems really solid from an API perspective, well done 👍! Now what we need is more tests to get this ready to be merged. Quite a few things did not work properly or not at all for me and tests would also help to document how certain things are meant to be used and provide an easy way to check out edge case work flows.

Not sure if I can manage to continue my review tomorrow, it will probably be Monday. Thanks for the hard work!

lukastaegert · 2018-05-26T19:37:37Z

src/Graph.ts

+					});
+				asset.source = source;
+			},
+			resolveId: undefined,


I know it is only a very minor thing but I'd like to have a test that uses this.

Sure will add one. There is a circular risk here too in calling this from the resolve hook itself, but I think that's just common sense to avoid.

lukastaegert · 2018-05-26T19:42:09Z

src/Graph.ts

+		};
+
+		this.pluginContext = {
+			isExternal: this.isExternal,


We should also have a test for this hook. In fact during the definition of the context, this.isExternal is still undefined. The simplest approach could be to wrap it in a function, otherwise you probably need to reorder things.

lukastaegert · 2018-05-26T19:49:08Z

src/Graph.ts

+
+		this.pluginContext = {
+			isExternal: this.isExternal,
+			emitAsset: (name: string, source?: string | Buffer) => {


I wonder what is the use case of defining an assets in a two-step process, i.e. first we register it and then at a later stage we set the source? And we can only set the source once?

The reason it's a two-step process is due to the order of information:

You may want to construct an asset (say a CSS file) to match the list of modules defined by a chunk, so you will only know the asset source after generate is called.

The chunk source itself might want to reference the asset URL via import.meta.ROLLUP_ASSET_URL_sd9f8fg0s, even though the asset source isn't yet known at this point.

Similarly, assets themselves might need to reference other assets, and only be generated late.

So the separation of assetId through the three methods allows us to dodge all these issues!

Ah I see. import.meta.ROLLUP_ASSET_URL was one of the few things I had not tested yet. That makes sense!

lukastaegert · 2018-05-26T19:55:01Z

src/utils/getAssetFileName.ts

+	if (source === undefined)
+		error({
+			code: 'MISSING_ASSET_SOURCE',
+			message: `Plugin error creating asset ${assetName} The asset source must always be provided in order to support hash file names for assets.`


There should probably be a "." after the asset name.

lukastaegert · 2018-05-26T20:07:54Z

src/utils/transform.ts

+				code: 'PLUGIN_WARNING',
+				id,
+				source,
+				pluginName: plugin.name


If a plugin does not have a name (which happens often for ad-hoc plugins) the warning will read: "(!) undefined plugin: ..." which is somewhat confusing. One approach could be to set the pluginName to "unnamed" which makes for less confusing warnings.

The same would apply to errors though in that case, the error message is less confusing (the message seems to assume this is not a plugin error). Still, I think if the error reads "[!] (unnamed plugin) Error: ...", this would provide the helpful information that the error did not originate from rollup itself but probably from some code the user wrote themselves.

Another issue I stumbled upon: When I generate warnings or errors from the transformBundle hook, I get a maximum callstack error instead.

lukastaegert · 2018-05-26T20:35:55Z

src/utils/transformChunk.ts

+		return promise.then(code => {
+			return Promise.resolve()
+				.then(() =>
+					(plugin.transformChunk || plugin.transformBundle).call(


I must admit I could not get transformChunk to be called at all in my experiments. A test to take some inspiration from would be really helpful.

lukastaegert · 2018-05-26T20:38:08Z

src/utils/transformChunk.ts

+	options: OutputOptions
+) {
+	return graph.plugins.reduce((promise, plugin) => {
+		if (!plugin.transformBundle) return promise;


Actually, this might be the reason why transformChunk does not work at all...

Ahh, thanks.

lukastaegert · 2018-05-26T20:43:54Z

src/rollup/index.ts

 	try {
 		const inputOptions = getInputOptions(rawInputOptions);
 		initialiseTimers(inputOptions);
 		const graph = new Graph(inputOptions);

-		timeStart('BUILD', 1);
+		for (let plugin of graph.plugins) {
+			if (plugin.buildStart) plugin.buildStart.call(graph.pluginContext, inputOptions);


Using this.warn in the buildStart and buildEnd hooks also caused maximum callstack errors. I think we should have a few more simple tests for the plugin context API. There are already some tests in the function category for this.warn that could probably be easily adapted for the other hooks.

guybedford · 2018-05-27T13:08:08Z

I'll integrate the feedback and some further changes I'd like to make on this tomorrow, so no need to review further before then.

guybedford · 2018-05-28T16:08:59Z

Thanks for testing out those cases - I've fixed and included tests for these now.

I also added support for even later asset emissions at the generateBundle phase, along with a getAssetFileName function.

Ready for further review!

lukastaegert

Thanks, works much better! Did not manage to do as much testing today as I would have liked, just two comments for now.

lukastaegert · 2018-05-29T06:36:41Z

src/rollup/index.ts


-		if (!codeSplitting) {
+		if (!inputOptions.experimentalCodeSplitting) {


Very nice and clean to have all these validations grouped together. I wonder if it would be possible to move these option validations out of the rollup function and into getInputOptions? @

lukastaegert · 2018-05-29T06:54:51Z

src/rollup/index.ts


 					timeStart('GENERATE', 1);

-					const generated: { [chunkName: string]: OutputChunk } = {};
+					// populate asset files into output
+					const assetFileNames = outputOptions.assetFileNames || 'assets/[name]-[hash][ext]';


It would be really nice if "[ext]" would not contain the "." as in this case, I could use a pattern such as "[ext]/[name].[ext]" i.e. grouping different types of assets into different folders.

The issue with this is that in theory we support assets without an extension, so the [ext] pattern can be the empty string. Perhaps we could introduce a new variable - [extname] or something like that?

Note we can always move to supporting a function for these patterns as well -

assetFileNames: (vars) => { if (vars.ext === '.css') return `assets/style/${vars.name}`; return `assets/${vars.name}`; }

Supporting a function would definitely be great though I would not strictly require it for the initial version. But of course the default value should support that plugins emit extensionless files which would not be possible with my approach.

I am still not 100% happy with this as I think it could be confusing for non-Linux-natives that the dot is part of the extension (it was for me). Having a second placeholder could actually solve this.

Also there does not seem to be any prior art from the likes of Webpack.

Note that this is the same output as the NodeJS path.extname, by inluding the dot.

Perhaps instead of [ext] it should be called [extname] with [ext] being the version without the dot? That might make it more obvious to Node users too.

Sounds good

lukastaegert · 2018-05-30T05:31:43Z

src/rollup/index.ts


 					timeStart('GENERATE', 1);

-					const generated: { [chunkName: string]: OutputChunk } = {};
+					// populate asset files into output
+					const assetFileNames = outputOptions.assetFileNames || 'assets/[name]-[hash][ext]';


Supporting a function would definitely be great though I would not strictly require it for the initial version. But of course the default value should support that plugins emit extensionless files which would not be possible with my approach.

I am still not 100% happy with this as I think it could be confusing for non-Linux-natives that the dot is part of the extension (it was for me). Having a second placeholder could actually solve this.

Also there does not seem to be any prior art from the likes of Webpack.

lukastaegert · 2018-05-30T06:13:38Z

test/hooks/index.js

+			});
+	});
+
+	it('supports warnings in buildStart and buildEnd hooks', () => {


This appears to be a duplicate

lukastaegert · 2018-05-30T06:51:10Z

test/hooks/index.js

+			.rollup({
+				input: 'input',
+				experimentalCodeSplitting: true,
+				experimentalDynamicImport: true,


acorn will only parse import.meta if experimentalDynamicImport is true, otherwise it will produce an "unexpected token" error. Since we want this asset emission API to be used by plugins today, I guess this needs to be the PR where we make this feature no longer experimental and always add the acorn plugin.

Alternatively, we could just make sure the plugin for import.meta is always added independent of the flag.

I think we can unflag experimentalDynamicImport, but I'm still weary of the dynamic import hook being documented.

lukastaegert · 2018-05-30T07:06:13Z

test/hooks/index.js

+			});
+	});
+
+	it('supports CommonJS asset urls', () => {


I tried this for amd and got something like

define(function () { 'use strict'; console.log(new URL((typeof process !== 'undefined' && process.versions && process.versions.node ? 'file:' : '') + module.uri + '/../assets/asset.ext').href); });

I would have expected module to be imported by the amd loader similar to how it is done for import.meta.url.

Also, system is not supported at all as a format but throws in a rather non-pleasing way with a TypeError.

Thanks, will fix.

guybedford · 2018-05-30T10:56:45Z

I've updated these now. In adding tests for the System output I caught a magic string bug, which this now depends on at Rich-Harris/magic-string#144.

guybedford · 2018-05-30T12:08:45Z

I've tweaks the hooks slightly with the following additional changes (updated in the PR summary as well):

buildStart and buildEnd now support promises
processChunks is removed
generateBundle now gets a chunk with a "modules" property that is no longer an array of moduleIds but instead an object of { [moduleId: string]: { renderedExports: string[], removedExports: string[], originalLength: number, renderedLength: number } } allowing inspection into the treeshaking improvements.

Note that the renderedLength doesn't include the Rollup wrapper or imports or exports as well, so will always be less and may be slightly deceving (eg export var p = 5; is reported as a length of 10. We could potentially have a wrapperLength property on the chunk itself to correct this.

I also created Rich-Harris/magic-string#145 for this to try optimize getting the length a bit.

lukastaegert · 2018-05-31T11:02:16Z

generateBundle now gets a chunk with a "modules" property that is no longer an array of moduleIds but instead an object of { [moduleId: string]: { renderedExports: string[], removedExports: string[], originalLength: number, renderedLength: number } } allowing inspection into the treeshaking improvements

I love this very much. This could be the first step to enable a whole now class of analysis and visualization plugins.

lukastaegert · 2018-05-31T11:48:55Z

src/ast/nodes/ExportDefaultDeclaration.ts

@@ -159,7 +159,7 @@ export default class ExportDefaultDeclaration extends NodeBase {
 		);
 		const hasTrailingSemicolon = code.original.charCodeAt(this.end - 1) === 59; /*";"*/
 		if (systemBinding) {
-			code.appendLeft(


I have thought a long time about this. So the reason you were having troubles is because the code directly to the left your insertion was overwritten which signals to magic-string to remove everything appended to it.

So the appendRight now either associates the inserted code with the semicolon or with the next white-space. I think it is not possible to have an export default declaration without some trailing white-space or semi-colon. So your fix should always work and I do not have a better idea.

But you should change the appendLeft in the alternate case of the if statement as well.

It's particularly the system binding case that suffers from the double write issue, where we want to add an appendage first, then later on rewrite the inner content. It might be more obvious think in terms of overwriteInner and overwriteOuter here, see Rich-Harris/magic-string#144 (comment) for further comment on this.

lukastaegert · 2018-05-31T12:06:41Z

src/rollup/index.ts

-							);
-						});
+				// TODO: deprecate legacy single chunk return
+				let singleInputChunk: Chunk;


This should be an optional type

lukastaegert · 2018-05-31T12:10:43Z

src/rollup/index.ts

+					) {
+						singleInputChunk = chunks.find(chunk => chunk.entryModule !== undefined);
+						imports = singleInputChunk.getImportIds();
+						exports = singleInputChunk.getExportNames();


Maybe instead of defining imports and exports here and then use them the next time more than 200 lines later (with other variables of the same name in between) we can define them around line 365 and only add them specifically in the single input chunk case?

At one point I was sharing this to avoid duplicate work, but now that it isn't that makes sense, thanks.

lukastaegert · 2018-05-31T12:23:19Z

src/rollup/index.ts

-							message:
-								'UMD and IIFE output formats are not supported with the experimentalCodeSplitting option.'
-						});
+					if (inputOptions.experimentalCodeSplitting) {


So as far as I can see, it should be possible to do single file builds with experimentalCodeSplitting turned on. So could we make it the default soon or are there still major things missing?

Really great work refactoring this though I am not sure I will catch everything here.

Exactly, I've turned it into a flag in this PR that we can then just disable when we are ready.

In terms of a checklist for launch, I think it would just be nice to have a little bit more feedback on usage patterns, to give us a bit more space to not worry about any unexpected API churn for a little while.

But I do think this gets us to almost stable... think of it as a release candidate.

The other thing to note is that adding the flag is a breaking change in that imports, exports and modules as properties of the build bundle before generate are no longer present.

Ah I see, yes that would be a breaking change. So maybe we keep it at least for the 0.60.0 and then remove it for the 1.0.0. One thing I keep wondering about is if it is a wise idea to directly return a map of the chunks from generate. This means we will never be able to add any other meta-information that is not chunk-specific here. Maybe instead we could return an object
{chunks: {[chunk]: <chunk-data>}} here? That would leave the door open for extensions.

One thing we could do for the 0.60.0, however, is to change all our tests to use experimentalCodeSplitting and thus test the new code paths. Should be a great way to find lingering bugs and inconsistencies. I fear I will not be able to finish the review this week as I had planned as I will be on a hiking trip until Sunday but it looks really great!

lukastaegert

Looks really good. There are still a few things I want to check which is playing around with inlined imports and having a look at tests which will unfortunately have to wait until Monday.

lukastaegert · 2018-05-31T19:24:50Z

test/function/samples/import-meta-url-compact/_config.js

@@ -0,0 +1,20 @@
+const assert = require('assert');
+const path = require('path');
+const URL = require('url-parse');


url-parse needs to be added to rollup's dev dependencies. This is why tests on CI have been failing for some time.

lukastaegert · 2018-05-31T19:48:54Z

src/rollup/index.ts

-							message:
-								'UMD and IIFE output formats are not supported with the experimentalCodeSplitting option.'
-						});
+					if (inputOptions.experimentalCodeSplitting) {


Ah I see, yes that would be a breaking change. So maybe we keep it at least for the 0.60.0 and then remove it for the 1.0.0. One thing I keep wondering about is if it is a wise idea to directly return a map of the chunks from generate. This means we will never be able to add any other meta-information that is not chunk-specific here. Maybe instead we could return an object
{chunks: {[chunk]: <chunk-data>}} here? That would leave the door open for extensions.

lukastaegert · 2018-05-31T19:53:51Z

src/rollup/index.ts

-							message:
-								'UMD and IIFE output formats are not supported with the experimentalCodeSplitting option.'
-						});
+					if (inputOptions.experimentalCodeSplitting) {


One thing we could do for the 0.60.0, however, is to change all our tests to use experimentalCodeSplitting and thus test the new code paths. Should be a great way to find lingering bugs and inconsistencies. I fear I will not be able to finish the review this week as I had planned as I will be on a hiking trip until Sunday but it looks really great!

guybedford · 2018-06-01T16:10:01Z

I've adjusted the return value of generate and write to be { output: { [fileName: string]: string | Buffer | Chunk } }, so it can be extensible in future.

Good idea on switching the tests to experimentalCodeSplitting for 0.60. Do you want to aim to include that in this PR as well?

Added url-parse as a dependency now, hopefully that does fix the tests, thanks was wondering what was up with that.

lukastaegert · 2018-06-04T10:37:05Z

While playing around with the dynamic import inlining, I discovered the following bug (which actually seems to have existed for some time):

// foo.js
export const foo = 1;

// main.js
import { foo } from './foo.js';
export const bar = 2
import('./foo.js')

// output when bundling main.js
const foo = 1;

var foo$1 = /*#__PURE__*/Object.freeze({
	foo: foo
});

const bar = 2;
Promise.resolve().then(function () { return foo$1; });

export { foo };

Note that the bundle is actually exporting foo, not bar i.e. the exports of main.js seem to have been replaced by the exports of foo.js. If you remove the non-dynamic import of foo.js, bar is correctly exported. Did not dig deeper into this yet.

lukastaegert · 2018-06-04T11:56:05Z

I've adjusted the return value of generate and write
👍

Good idea on switching the tests to experimentalCodeSplitting for 0.60. Do you want to aim to include that in this PR as well?

Considering that I would love to have this PR in 0.60.0 and that there are now some important fixes in master that should be released soon, I would actually postpone this. Once this is released, we could just do the actual switch to remove the flag and would not have to edit all config files twice.

If you could have a look at the bug I outlined above, that would be really cool, though. Seems to be more like your area of expertise but I could have a look later myself as well.

…fo in generateBundle modules

guybedford · 2018-06-04T21:29:09Z

Thanks for spotting the inlineDynamicImports bug, I've included the fix here in the latest commit - it was misdetecting the entry point module of the chunk, much more straightforward than it seemed :)

lukastaegert

Looks good! If there are no objections form your side, I will merge it tomorrow morning and release 0.60.0.

guybedford requested a review from lukastaegert May 22, 2018 23:42

lukastaegert reviewed May 25, 2018

View reviewed changes

guybedford force-pushed the plugin-chunking branch 2 times, most recently from 9ec757d to fa62c2e Compare May 26, 2018 13:53

lukastaegert reviewed May 26, 2018

View reviewed changes

guybedford force-pushed the plugin-chunking branch from fa62c2e to 1dcec6d Compare May 27, 2018 13:07

lukastaegert mentioned this pull request May 28, 2018

Expanding input's array- and object-form beyond experimentalCodeSplitting? #2227

Closed

guybedford force-pushed the plugin-chunking branch from 1dcec6d to 53805de Compare May 28, 2018 15:28

lukastaegert reviewed May 29, 2018

View reviewed changes

lukastaegert reviewed May 30, 2018

View reviewed changes

lukastaegert mentioned this pull request May 30, 2018

Add rendered length to module #2229

Closed

guybedford force-pushed the plugin-chunking branch from c15837d to 429b55e Compare May 30, 2018 10:55

lukastaegert reviewed May 31, 2018

View reviewed changes

tivac mentioned this pull request May 31, 2018

chore: allow building/testing on cmd #2232

Merged

lukastaegert approved these changes May 31, 2018

View reviewed changes

lukastaegert added this to the 0.60.0 milestone Jun 4, 2018

guybedford added 6 commits June 4, 2018 19:26

plugin api ajustments

a329517

unified plugin interface, pluginContext

9e8bbf7

typing fixes

555259c

add onbuildstart and onbuildend hooks

b00204a

sync-only onbuildstart, onbuildend

f49dd00

onbuilderror hook

3aace48

guybedford added 6 commits June 4, 2018 19:27

remove processChunks hook for generateBundle, include tree-shaking in…

de83598

…fo in generateBundle modules

use MagicString.length()

55ac656

singleInputChunk tweaks

7232bec

update magic-string

ee53a31

add url parse dependency

994bd6e

provide bundle on output property

2a634c7

guybedford force-pushed the plugin-chunking branch 2 times, most recently from 0418fbb to 4160d25 Compare June 4, 2018 17:38

use native URL in tests if possible

b9f085a

guybedford force-pushed the plugin-chunking branch from 4160d25 to b9f085a Compare June 4, 2018 17:42

tweak dir / file error message cases

649a046

guybedford force-pushed the plugin-chunking branch from ea21780 to 649a046 Compare June 4, 2018 20:10

fix inlineDynamicImports entry misrecognition bug

a7058a6

guybedford force-pushed the plugin-chunking branch from eabff85 to a7058a6 Compare June 4, 2018 21:30

lukastaegert approved these changes Jun 5, 2018

View reviewed changes

lukastaegert merged commit a7058a6 into master Jun 6, 2018

lukastaegert added the fixed pending release label Jun 6, 2018

lukastaegert deleted the plugin-chunking branch June 6, 2018 05:31

lukastaegert removed the fixed pending release label Jun 6, 2018

This was referenced Jun 6, 2018

plugin init or reset hook #1611

Closed

add onerror hook or something like that #1629

Closed

this.warn and this.error are not available in transformBundle #1282

Closed

Give plugins bundle access? #803

Closed

tivac mentioned this pull request Jun 7, 2018

feat: add support for Rollup 60 tivac/modular-css#429

Merged

lukastaegert mentioned this pull request Jun 24, 2018

Pass codeSplitting generated filenames to onwrite hooks #2115

Closed

This was referenced Aug 10, 2018

Terser or Uglify doesn't execute inside a Conditional AgronKabashi/rollup-plugin-conditional#3

Closed

Clarify transformBundle deprecation #2395

Closed

guybedford mentioned this pull request Aug 13, 2018

renderChunk hook to replace transformChunk #2406

Merged

8 tasks

vladshcherbin mentioned this pull request Apr 30, 2019

Add sequential before build hook #2826

Open


		if (!codeSplitting) {
		if (!inputOptions.experimentalCodeSplitting) {

Build and plugin hooks refactoring for code splitting and asset emission workflows #2208

Build and plugin hooks refactoring for code splitting and asset emission workflows #2208

Conversation

guybedford commented May 22, 2018 • edited

Refactoring Summary (for the third time...):

Asset Emission Features

lukastaegert left a comment

Choose a reason for hiding this comment

lukastaegert May 25, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lukastaegert left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

guybedford commented May 27, 2018

guybedford commented May 28, 2018

lukastaegert left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

guybedford May 29, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

guybedford commented May 30, 2018

guybedford commented May 30, 2018

lukastaegert commented May 31, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lukastaegert left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

guybedford commented Jun 1, 2018

lukastaegert commented Jun 4, 2018

lukastaegert commented Jun 4, 2018

guybedford commented Jun 4, 2018

lukastaegert left a comment

Choose a reason for hiding this comment

guybedford commented May 22, 2018 •

edited

lukastaegert May 25, 2018 •

edited

guybedford May 29, 2018 •

edited