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

Add Chisel logo to repo and README #930

Merged
merged 1 commit into from
Nov 9, 2018

Conversation

albert-magyar
Copy link
Contributor

@seldridge @ucbjrl , this should make it easier for people to find.

@albert-magyar albert-magyar requested a review from a team as a code owner November 8, 2018 21:00
Copy link
Contributor

@chick chick left a comment

Choose a reason for hiding this comment

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

lgtm

@edwardcwang
Copy link
Contributor

Can we have less spacing around the logo, or put it under the header?

@albert-magyar albert-magyar force-pushed the logo branch 8 times, most recently from cd64138 to 1f0ea2d Compare November 8, 2018 21:49
@albert-magyar
Copy link
Contributor Author

albert-magyar commented Nov 8, 2018

I switched to a cropped SVG from @seldridge, which will fix that space @edwardcwang. I also reverted the README changes; this way, I can add them back by referencing a raw Github content link directly to the vector version.

@seldridge
Copy link
Member

seldridge commented Nov 8, 2018

Looks good. Anecdotally, I like the rebase/amend to get it down to one commit. However, at least with Travis, the jobs started in the rebase don't get killed. Jenkins may be doing the same thing here. (The implication being that the build queue gets very full.)

Also, what happened to CircleCI? @ucbjrl, I don't think it was turned off?

@albert-magyar
Copy link
Contributor Author

Yeah, the real problem is that I kept pushing to my branch after I opened the PR to check the README formatting :(

@ucbjrl
Copy link
Contributor

ucbjrl commented Nov 8, 2018

I think the issue is this is coming from a repo outside of freechipsproject. I'm not sure what needs to happen to get CircleCI to do its thing. I suspect this is a CircleCI bug.

@ucbjrl
Copy link
Contributor

ucbjrl commented Nov 8, 2018

It looks like there's a setting in the CircleCI project which needs to be enabled to allow builds for pull requests from forks. I've enabled it for freechipsproject. Let's see if it requires another push to trigger.

@ucbjrl
Copy link
Contributor

ucbjrl commented Nov 8, 2018

I tried redelivering the notification from GitHub but that doesn't seem effective. @albert-magyar could you push a gratuitous change to see if that will trigger CircleCI?

@albert-magyar
Copy link
Contributor Author

Yeah, it did trigger it @ucbjrl

@seldridge seldridge merged commit aa3100c into chipsalliance:master Nov 9, 2018
@ucbjrl ucbjrl added this to the 3.1.4 milestone Nov 30, 2018
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.

5 participants