Skip to content

Conversation

@fregante
Copy link
Member

@fregante fregante commented Nov 6, 2017

CI build status

multiple CI status

Notes:

Testable pages:

Fixes #726

@fregante
Copy link
Member Author

fregante commented Nov 6, 2017

Ready for testing now. 👍

@sindresorhus
Copy link
Member

Very cool and seems to work great.

@sindresorhus
Copy link
Member

Quite an obscure bug, but when I created a new PR and it redirected to the created PR, I got this:

screen shot 2017-11-07 at 02 16 22

@fregante
Copy link
Member Author

fregante commented Nov 7, 2017

Somehow it probably got to make two fetches before one was complete since there’s already a select.exists that should prevent this.

@sindresorhus
Copy link
Member

I couldn't reproduce it when I did another PR, so probably some kind of obscure race condition indeed.

+ Prevent quick repeated executions from piling up
@fregante
Copy link
Member Author

fregante commented Nov 7, 2017

That should do it. I just cache the entire fetch + select in one Promise.

@sindresorhus
Copy link
Member

Can you add it to the readme?

@fregante fregante changed the title Add CI link next to repo Add build status and link to CI next to the repo's title Nov 8, 2017
@fregante fregante merged commit 457071f into master Nov 8, 2017
@yakov116
Copy link
Member

yakov116 commented Nov 8, 2017

@bfred-it I guess its not intended to work on private repo's right?

@fregante fregante deleted the ci-link branch November 8, 2017 17:28
@fregante
Copy link
Member Author

fregante commented Nov 8, 2017

You're 20 minutes too late! I think I was just missing credentials: 'include' in the fetch. I'll try

@yakov116
Copy link
Member

yakov116 commented Nov 8, 2017

@bfred-it I don't normally test the pr's unless I reported the issue

@fregante
Copy link
Member Author

fregante commented Nov 8, 2017

Done in 04d32b7

status on private repo


async function fetchStatus() {
const url = `${location.origin}/${getRepoURL()}/commits/`;
const dom = await fetch(url).then(r => r.text()).then(domify);
Copy link
Member

Choose a reason for hiding this comment

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

@bfred-it This could be:

const response = await fetch(url);
const dom = domify(await response.text());

And here too:

src/content.js
88:	domLoaded.then(onDomReady);

src/libs/synchronous-storage.js
30:		return get().then(value => {

src/libs/utils.js
36:	domLoaded.then(() => requestAnimationFrame(() => waiting.cancel()));

src/libs/api.js
3:	return fetch(api + endpoint).then(res => res.json());

src/features/linkify-branch-refs.js
33:	safeElementReady('.branch-name').then(el => {

src/features/show-names.js
16:		.then(res => res.text());

src/features/show-recently-pushed-branches.js
18:	}).then(res => res.text());

src/features/mark-unread.js
335:			}).then(storage => storage.unreadNotifications);

src/features/move-account-switcher-to-sidebar.js
5:	safeElementReady('.dashboard-sidebar').then(sidebar => {

Copy link
Member Author

Choose a reason for hiding this comment

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

Done in #814

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants