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

fix #59, add information crawler #211

Closed
wants to merge 6 commits into from

Conversation

DerekNonGeneric
Copy link
Contributor

@DerekNonGeneric DerekNonGeneric commented Sep 12, 2020

Still need to determine where to reliably locate code samples and descriptions, but this is nearly done. 🎉

I happened to run across this old gh-file-importer.mjs module I wrote a few months ago (still needs revision, but works) and immediately thought about #199. It's still early in the semester so I'm not buried in assignments yet, so sorry for stacking the PRs, but this opportunity doesn't come too often.

Since I can't reliably locate code samples, I'm currently just parsing the Markdown and using the last JavaScript code fence that appears on the page. There does not seem to be a designated place to look for proposal descriptions, so I'm just using lorem ipsum until we can find a better way to do this.

I must say, I severely underestimated this task and am glad that I went for it when I did. Run it via npm run sync:proposal-data and you will see it in action. Once it runs once, the _data/stage3.yml file gets updated with the latest info, which can be observed locally once the site is built using this data. I know we still need to integrate it into the CI as suggested in #30, but early feedback on this PR would be amazing. I'm especially concerned about the logging aspect of this. I left several debug log statements commented out to assist with review, but am still unsure what logging should be occurring while the task runs to completion.

Please keep me posted with suggestions!

/cc @codehag @ljharb

@DerekNonGeneric DerekNonGeneric force-pushed the feat/info-crawler branch 6 times, most recently from 1e0f530 to 9cd437a Compare September 12, 2020 18:48
Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

General comment/question about all the .catch-without-rethrows all over the place?

_tasks/sync-proposal-data.mjs Outdated Show resolved Hide resolved
_tasks/sync-proposal-data.mjs Outdated Show resolved Hide resolved
_tasks/sync-proposal-data.mjs Outdated Show resolved Hide resolved
_tasks/sync-proposal-data.mjs Outdated Show resolved Hide resolved
.nvmrc Outdated Show resolved Hide resolved
_data/stage3.yml Outdated Show resolved Hide resolved
_tasks/sync-proposal-data.mjs Outdated Show resolved Hide resolved
_tasks/sync-proposal-data.mjs Outdated Show resolved Hide resolved
_tasks/sync-proposal-data.mjs Outdated Show resolved Hide resolved
_tasks/sync-proposal-data.mjs Outdated Show resolved Hide resolved
_tasks/util/gh-file-importer.mjs Outdated Show resolved Hide resolved
_tasks/util/gh-file-importer.mjs Outdated Show resolved Hide resolved
_tasks/util/gh-file-importer.mjs Outdated Show resolved Hide resolved
_tasks/util/gh-file-importer.mjs Outdated Show resolved Hide resolved
Comment on lines 301 to 283
// TODO(DerekNonGeneric): Determine where to reliably locate descriptions.
const /** @type {string} */ description = 'Lorem ipsum.';
hashmap.set('description', description);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was thinking that using the description of the GitHub repo would be acceptable, but that doesn't seem to be the currently followed practice. It's possible that these one-line descriptions are all located in the same place for each proposal and I haven't noticed where to look yet. To illustrate my point, take proposal-static-class-features for example.

Current proposal description in stage3.yml:

This proposal adds Static public fields, Static private methods and Static private fields

Current proposal description in GitHub repo UI:

The static parts of new class features, in a separate proposal


Currently, parsing each proposal's README for a one-line description would be complicated since these descriptions seem to be derived from non-standard locations in the document, so that seems like a no-go.

@codehag
Copy link
Collaborator

codehag commented Sep 14, 2020

@DerekNonGeneric Wow fantastic work. This was a great surprise to see this morning. I haven't had a chance to dig deeply into this yet, I will try to do that later today or tomorrow. I just wanted to say early -- Thank you so much for the work you are doing. It is very much appreciated.

Review incoming.

@DerekNonGeneric DerekNonGeneric force-pushed the feat/info-crawler branch 3 times, most recently from 9d1b3a1 to 127e90b Compare September 14, 2020 18:39
@DerekNonGeneric
Copy link
Contributor Author

Hi @codehag, glad to hear you're pleased with the direction this is headed in!

I haven't had a chance to dig deeply into this yet, I will try to do that later today or tomorrow.

I've been able to clean it up a bit since the initial PR, so you probably dodged having to review even worse code. :)

Looking forward to initial thoughts and suggestions, I know there's still a bit more left until this would be considered complete.

_tasks/sync-proposal-data.mjs Outdated Show resolved Hide resolved
_tasks/util/gh-file-importer.mjs Outdated Show resolved Hide resolved
@DerekNonGeneric DerekNonGeneric force-pushed the feat/info-crawler branch 6 times, most recently from 9f79694 to 9cd437a Compare September 16, 2020 15:28
@codehag
Copy link
Collaborator

codehag commented Sep 16, 2020

Sorry I haven't had a chance to look at this yet. So no worries -- keep working as you are.

The mozilla work flow won't apply here. We can treat this as a regular repository.

@DerekNonGeneric DerekNonGeneric force-pushed the feat/info-crawler branch 3 times, most recently from be3e26f to 6672e0f Compare September 16, 2020 23:42
@septs

This comment has been minimized.

@DerekNonGeneric DerekNonGeneric force-pushed the feat/info-crawler branch 2 times, most recently from 088c40b to b7f7b12 Compare November 7, 2020 01:41
@DerekNonGeneric DerekNonGeneric force-pushed the feat/info-crawler branch 2 times, most recently from a4c92d5 to a7753cd Compare November 20, 2020 04:57
_data/stage3.yml Outdated Show resolved Hide resolved
@tc39 tc39 deleted a comment from ljharb Dec 9, 2020
@tc39 tc39 deleted a comment from ljharb Dec 9, 2020
@DerekNonGeneric DerekNonGeneric marked this pull request as ready for review December 9, 2020 04:29
@DerekNonGeneric DerekNonGeneric marked this pull request as draft December 11, 2020 16:38
@DerekNonGeneric DerekNonGeneric force-pushed the feat/info-crawler branch 3 times, most recently from 82a99e4 to e342b80 Compare February 2, 2021 04:45
@DerekNonGeneric DerekNonGeneric marked this pull request as ready for review February 2, 2021 04:50
@smorimoto
Copy link
Member

This seems to be inactive for quite some time. Could you open a new PR as needed?

@smorimoto smorimoto closed this Nov 4, 2023
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

Successfully merging this pull request may close these issues.

6 participants