Skip to content
This repository has been archived by the owner on Oct 21, 2020. It is now read-only.

Add package size #386

Closed
wants to merge 8 commits into from
Closed

Add package size #386

wants to merge 8 commits into from

Conversation

styfle
Copy link
Contributor

@styfle styfle commented Jun 12, 2018

Fixes #359

Copy link
Contributor

@erquhart erquhart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Couple of issues, one of which is breaking the build. I can review again once addressed and a deploy preview is live.

reject(error);
});
});
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could shrink this a lot by using fetch.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you already have this as a dependency or do I need to add node-fetch?

const npmPackageData = await getAllProjectPackageSizes(npms);

const data = projects.reduce((obj, { key, repo, npm, twitter }) => {
const twitterData = twitter ? { followers: twitterFollowers[twitter] }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing the or case for this ternary.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, I was falling asleep while writing this and couldn't figure out what I did wrong haha. I'll fix this when I get home.

@erquhart
Copy link
Contributor

I actually wouldn't put this metric in the stats, maybe a "Package size" item in the list below?

@styfle
Copy link
Contributor Author

styfle commented Jun 13, 2018

That was my first thought too but I don't know how to put it in the list below 😅

I just see the fields array but I'm not sure how to add a field in there 🤔

@erquhart
Copy link
Contributor

erquhart commented Jun 13, 2018

It's actually pulled straight from the individual project frontmatters:

const siteConfig = yaml.safeLoad(siteYaml)

staticgen/static.config.js

Lines 155 to 161 in acf13dc

getData: () => ({
projects,
promo,
sorts: siteConfig.sorts,
filters: generateFilters(projects),
fields: siteConfig.fields,
}),

It doesn't have to be just frontmatter though, you can add your data to the fields array right there in static.config.js.

@styfle
Copy link
Contributor Author

styfle commented Jun 15, 2018

Ok I see where fields come from: the site.yml file.
But where do the fieldValues come from?
And where should I add this new value?

@erquhart
Copy link
Contributor

Right here:

const fieldValues = pick(project, map(siteConfig.fields, 'name'))

I'd say create a new getFields function that combines siteConfig.fields with additional fields like yours, then call that function in place of siteConfig.fields throughout.

To get your values into fieldValues, in that same getProjects function, you can map additional values onto projectDetails after this line:

const projectDetails = map(projectsMarkdown, processMarkdown)

@styfle
Copy link
Contributor Author

styfle commented Jun 20, 2018

Ok I tried a couple things but I'm still stuck 😭
If you can make that change on this branch, feel free to do so 👍
I won't have time to make another attempt for a week or so.

@erquhart
Copy link
Contributor

Hey @styfle, just now circling back to this. I don't have much bandwidth to dig in - do you still plan on working on this?

@styfle
Copy link
Contributor Author

styfle commented Aug 22, 2018

Oops, I almost forgot about it 😅
I will try again this weekend and see if I can conquer it ⛰

@erquhart
Copy link
Contributor

@styfle closing this, but feel free to reopen if you ever circle back!

@erquhart erquhart closed this Feb 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants