Skip to content

Comments

Automatically convert MODXXX -> MOD#7729

Merged
dlang-bot merged 2 commits intodlang:masterfrom
wilzbach:MOD
Jan 17, 2018
Merged

Automatically convert MODXXX -> MOD#7729
dlang-bot merged 2 commits intodlang:masterfrom
wilzbach:MOD

Conversation

@wilzbach
Copy link
Contributor

replacements=(
"MODconst const_"
"MODimmutable immutable_"
"MODshared shared_"
"MODwild wild"
"MODwildconst wildconst"
"MODmutable mutable"
)
shopt -s globstar
for r in "${replacements[@]}" ; do
    w=($r)
    sed "s/${w[0]}/MODFlags.${w[1]}/g" -i **/*.d
done

@dlang-bot
Copy link
Contributor

Thanks for your pull request, @wilzbach!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

@PetarKirov
Copy link
Member

You also need to update:

dmd/src/dmd/mtype.h

Lines 116 to 125 in 254c963

enum MODFlags
{
MODconst = 1, // type is const
MODimmutable = 4, // type is immutable
MODshared = 2, // type is shared
MODwild = 8, // type is wild
MODwildconst = (MODwild | MODconst), // type is wild const
MODmutable = 0x10 // type is mutable (only used in wildcard matching)
};
typedef unsigned char MOD;

@WalterBright
Copy link
Member

WalterBright commented Jan 16, 2018

Can we slow down on this stuff? All this refactoring means I spend many hours rebasing and I presume everyone else, too.

@PetarKirov
Copy link
Member

@WalterBright On the contrary, I think we should hurry up and finish all of them and be done with this particular nuisance once and for all. As you can see here, most of the work is already done:
https://github.com/dlang/dmd/projects/4#card-6718852

If we complete this task quickly it means that most PRs will have to be rebased only once, and if we spread the work slowly over time it means that many PRs will need to be rebased multiple times.

@wilzbach
Copy link
Contributor Author

Can we slow down on this stuff? All this refactoring means I spend many hours rebasing

Where do you spend it? Are you referring to #6907 (comment)? I rebased this PR within five minutes after you posting there. Please don't hesitate to ping me if you run into. I'm more than happy to help out with rebasing any PR.

I presume everyone else, too.

We actively go through the PR queue and rebase or recreate old PRs that have value. Very often, however, it's:

git checkout pr/XXXX
git rebase upstream/master
pr_push

If we complete this task quickly it means that most PRs will have to be rebased only once, and if we spread the work slowly over time it means that many PRs will need to be rebased multiple times.

That was exactly my motivation for finishing this up.

```bash
replacements=(
"MODconst const_"
"MODimmutable immutable_"
"MODshared shared_"
"MODwild wild"
"MODwildconst wildconst"
"MODmutable mutable"
)
shopt -s globstar
for r in "${replacements[@]}" ; do
    w=($r)
    sed "s/${w[0]}/MODFlags.${w[1]}/g" -i **/*.d
done
```
@wilzbach wilzbach force-pushed the MOD branch 2 times, most recently from 5fc9e8e to 5715702 Compare January 16, 2018 20:08
@dlang-bot dlang-bot merged commit 7674cde into dlang:master Jan 17, 2018
@WalterBright
Copy link
Member

Where do you spend it?

There were a number of changes, such as code moving from one file to another, layered in with a few other commits that changed things in the changed sections, meaning the rebase needed a lot of manual work. They all failed the autotester, meaning I needed to track down if it was a rebase mistake or something else, all of which takes time as the autotester has a largish turnaround time.

@wilzbach wilzbach deleted the MOD branch January 18, 2018 02:10
@wilzbach
Copy link
Contributor Author

meaning the rebase needed a lot of manual work. They all failed the autotester, meaning I needed to track down if it was a rebase mistake or something else

FYI: dlang-bot marks PRs needing a rebase with "needs rebase". Whenever I have time, I browse the first page and see to it that everything is nicely green and rebased. Now that Travis has been replaced by SemaphoreCI we should start seeing green a lot more frequent.

I don't actively look at the older PRs as they can be rebased once they are actively approved, but please let me know if you have old PRs that need rebasing. My rebasing pipeline is pretty optimized ;-)

Also I'm pretty sure that I'm not the only one who would gladly help you with rebasing if that means less vetos on refactorings in the frontend.

If we complete this task quickly it means that most PRs will have to be rebased only once, and if we spread the work slowly over time it means that many PRs will need to be rebased multiple times.

Yes I'm done with this enum conversion series. Anything else that we should do in this refactoring phase?

@jacob-carlborg
Copy link
Contributor

Anything else that we should do in this refactoring phase?

There's a whole bunch of symbols that I would like to rename for clarity and following the D style guide. But that might be a different phase.

@jacob-carlborg
Copy link
Contributor

jacob-carlborg commented Jan 18, 2018

They all failed the autotester, meaning I needed to track down if it was a rebase mistake or something else, all of which takes time as the autotester has a largish turnaround time.

@WalterBright I do hope that you do some form of testing locally before pushing the code to let the autotester run.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants