Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Implement storing runtime state in repo level Git config#295

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged

Conversation

webknjaz
Copy link
Contributor

@webknjazwebknjaz commentedNov 20, 2018
edited
Loading

Ref#277

@webknjaz
Copy link
ContributorAuthor

@Mariatta@abadger I've cleaned things up a bit. Maybe it's time to move helper functions into util module/package or is it a task for separate PR?

@webknjaz
Copy link
ContributorAuthor

webknjaz commentedNov 27, 2018
edited
Loading

Hi@Mariatta,

I'm currently waiting for your approval wrt architectural changes. After that I'll proceed with adjusting/adding tests.

P.S.@abadger suggested that I could add a CLI command forgit config --local --remove-section cherry-picker but I'm doubtful: it's a dangerous thing for the flow and providing users with such UI might encourage them to overuse it where they don't understand consequences. Ideally, that failure shouldn't be happening during the normal process at all.

@Mariatta
Copy link
Member

Sorry for the delay. Will take a look in the weekend.

webknjaz reacted with thumbs up emoji

@webknjaz
Copy link
ContributorAuthor

Cool, thanks :)

@webknjaz
Copy link
ContributorAuthor

Hey@Mariatta, any 🤔💭 on this so far?

@webknjaz
Copy link
ContributorAuthor

Hi@Mariatta, any comments?

@Mariatta
Copy link
Member

So sorry, I think I'm not able to effectively review this PR.
Is there anyone else who might be able to help review this?

@webknjaz
Copy link
ContributorAuthor

@Mariatta
Well,@abadger said that it's fine. We just wanted to check that it's fine with you since you're the maintainer.

@webknjaz
Copy link
ContributorAuthor

I'll ask@asvetlov to take a look as well, then, to have more eyes on it.

Copy link
Contributor

@asvetlovasvetlov left a comment

Choose a reason for hiding this comment

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

The idea looks interesting.
Would you add tests for it?
At least we have several ones intest.py already.

@webknjaz
Copy link
ContributorAuthor

Thanks for the review, Andrew! I just wanted to wait for architectural approval before investing some time into tests. Now I can proceed :)

Mariatta reacted with thumbs up emoji

@Mariatta
Copy link
Member

Thanks@webknjaz and@asvetlov. For me, as long as there is no change to how it works for CPython /miss-islington’s purpose, then I'm good with it.

webknjaz reacted with hooray emoji

@webknjaz
Copy link
ContributorAuthor

Great, thanks@Mariatta!

@webknjazwebknjazforce-pushed thebugfix/consistent-config-cherry-picker branch 3 times, most recently fromf4f8d67 toa207fb9CompareJanuary 7, 2019 18:37
@webknjazwebknjazforce-pushed thebugfix/consistent-config-cherry-picker branch from4b63d05 todda278dCompareFebruary 9, 2019 23:39
@webknjazwebknjazforce-pushed thebugfix/consistent-config-cherry-picker branch fromdda278d to1a5d76fCompareFebruary 9, 2019 23:52
@webknjaz
Copy link
ContributorAuthor

Update: with the recent commits I've hit 83% test coverage.

Mariatta reacted with hooray emoji

@webknjazwebknjaz changed the title[WIP] [PoC] Initial implementation of storing state in Git configImplement storing runtime state in repo level Git configFeb 10, 2019
@webknjaz
Copy link
ContributorAuthor

@Mariatta@abadger@asvetlov I believe this is ready for the final review :)

Co-Authored-By: webknjaz <wk.cvs.github@sydorenko.org.ua>
@Mariatta
Copy link
Member

It seems ok to me. Would@abadger or@asvetlov be able to do more thorough review?

@webknjaz
Copy link
ContributorAuthor

Thanks. I'll ping them. I think Andrew is away for a couple of days, though...

Mariatta reacted with thumbs up emoji

Copy link
Contributor

@asvetlovasvetlov left a comment

Choose a reason for hiding this comment

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

LGTM now

@Mariatta
Copy link
Member

Thanks@asvetlov and@webknjaz. If you could update the release notes and up the version, I can merge and get it deployed to PyPI.

@webknjaz
Copy link
ContributorAuthor

Sure, on it

@webknjaz
Copy link
ContributorAuthor

done!

@MariattaMariatta merged commitb4773c9 intopython:masterFeb 21, 2019
@Mariatta
Copy link
Member

Thanks! Will release soon!

webknjaz reacted with hooray emojiwebknjaz reacted with rocket emoji

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

@MariattaMariattaMariatta left review comments

@asvetlovasvetlovasvetlov approved these changes

Assignees
No one assigned
Labels
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

4 participants
@webknjaz@Mariatta@asvetlov@the-knights-who-say-ni

[8]ページ先頭

©2009-2025 Movatter.jp